Skip to content

Commit 7027c90

Browse files
qunaibitsteve-s
qunaibit
authored and
steve-s
committed
address comments
1 parent 9cad5fd commit 7027c90

File tree

2 files changed

+25
-44
lines changed

2 files changed

+25
-44
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/HashingStorage.java

Lines changed: 23 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
5858
import com.oracle.graal.python.builtins.objects.function.PKeyword;
5959
import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
60-
import com.oracle.graal.python.lib.GetNextNode;
60+
import com.oracle.graal.python.lib.PyIterNextNode;
6161
import com.oracle.graal.python.lib.PyObjectGetItem;
6262
import com.oracle.graal.python.lib.PyObjectGetIter;
6363
import com.oracle.graal.python.lib.PyObjectLookupAttr;
@@ -68,7 +68,6 @@
6868
import com.oracle.graal.python.nodes.attributes.LookupCallableSlotInMRONode;
6969
import com.oracle.graal.python.nodes.builtins.ListNodes.FastConstructListNode;
7070
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
71-
import com.oracle.graal.python.nodes.object.BuiltinClassProfiles.IsBuiltinObjectProfile;
7271
import com.oracle.graal.python.nodes.object.GetClassNode;
7372
import com.oracle.graal.python.runtime.ExecutionContext.IndirectCallContext;
7473
import com.oracle.graal.python.runtime.PythonContext;
@@ -160,20 +159,23 @@ protected static boolean hasIterAttrButNotBuiltin(Node inliningTarget, Object co
160159
return attr != PNone.NO_VALUE && !(attr instanceof PBuiltinMethod || attr instanceof PBuiltinFunction);
161160
}
162161

163-
@Specialization(guards = {"!isNoValue(arg)", "!isPDict(arg) || hasIterAttrButNotBuiltin(inliningTarget, arg, getClassNode, lookupIter)"})
162+
@Specialization(guards = {"!isNoValue(arg)", "!isPDict(arg) || hasIterAttrButNotBuiltin(inliningTarget, arg, getClassNode, lookupIter)"}, limit = "2")
164163
static HashingStorage updateArg(VirtualFrame frame, Object arg, PKeyword[] kwargs,
165164
@Bind("this") Node inliningTarget,
166165
@SuppressWarnings("unused") @Exclusive @Cached GetClassNode getClassNode,
167166
@SuppressWarnings("unused") @Exclusive @Cached(parameters = "Iter") LookupCallableSlotInMRONode lookupIter,
168167
@Exclusive @Cached PyObjectLookupAttr lookupKeysAttributeNode,
169168
@Exclusive @Cached ObjectToArrayPairNode toArrayPair,
170-
@Exclusive @Cached HashingStorageSetItem setHasihngStorageItem,
171-
@Exclusive @Cached HashingStorageAddAllToOther addAllToOther) {
169+
@Exclusive @Cached HashingStorageSetItem setItem,
170+
@Exclusive @Cached HashingStorageAddAllToOther addAllToOther,
171+
@Cached InlinedConditionProfile hasKwds) {
172172
Object keyAttr = lookupKeysAttributeNode.execute(frame, inliningTarget, arg, T_KEYS);
173173
ArrayBuilder<KeyValue> elements = toArrayPair.execute(frame, arg, keyAttr);
174174
HashingStorage storage = PDict.createNewStorage(elements.size() + kwargs.length);
175-
storage = addKeyValuesToStorage(frame, elements, storage, inliningTarget, setHasihngStorageItem);
176-
storage = addKeywordsToStorage(frame, kwargs, storage, inliningTarget, addAllToOther);
175+
storage = addKeyValuesToStorage(frame, elements, storage, inliningTarget, setItem);
176+
if (hasKwds.profile(inliningTarget, kwargs.length > 0)) {
177+
storage = addAllToOther.execute(frame, inliningTarget, new KeywordsStorage(kwargs), storage);
178+
}
177179
return storage;
178180
}
179181

@@ -197,15 +199,6 @@ public final HashingStorage unionCached(HashingStorage other, HashingStorageCopy
197199
return addAllToOther.executeCached(null, other, newStore);
198200
}
199201

200-
public static HashingStorage addKeywordsToStorage(VirtualFrame frame, PKeyword[] kwargs, HashingStorage storage,
201-
Node inliningTarget,
202-
HashingStorageAddAllToOther addAllToOther) {
203-
if (kwargs.length > 0) {
204-
return addAllToOther.execute(frame, inliningTarget, new KeywordsStorage(kwargs), storage);
205-
}
206-
return storage;
207-
}
208-
209202
@ValueType
210203
protected static final class KeyValue {
211204
final Object key;
@@ -219,22 +212,22 @@ private KeyValue(Object key, Object value) {
219212

220213
public static HashingStorage addKeyValuesToStorage(VirtualFrame frame, ArrayBuilder<KeyValue> elements, HashingStorage storage,
221214
Node inliningTarget,
222-
HashingStorageSetItem setHashingStorageItem) {
215+
HashingStorageSetItem setItem) {
223216
for (int i = 0; i < elements.size(); i++) {
224217
Object key = elements.get(i).key;
225218
Object value = elements.get(i).value;
226-
storage = setHashingStorageItem.execute(frame, inliningTarget, storage, key, value);
219+
storage = setItem.execute(frame, inliningTarget, storage, key, value);
227220
}
228221
return storage;
229222
}
230223

231224
public static HashingStorage addKeyValuesToStorage(VirtualFrame frame, PDict self, Object other, Object keyAttr,
232225
Node inliningTarget,
233226
ObjectToArrayPairNode toArrayPair,
234-
HashingStorageSetItem setHashingStorageItem) {
227+
HashingStorageSetItem setItem) {
235228
ArrayBuilder<KeyValue> elements = toArrayPair.execute(frame, other, keyAttr);
236229
HashingStorage storage = self.getDictStorage();
237-
return addKeyValuesToStorage(frame, elements, storage, inliningTarget, setHashingStorageItem);
230+
return addKeyValuesToStorage(frame, elements, storage, inliningTarget, setItem);
238231
}
239232

240233
// partial impl dict_update_arg
@@ -252,23 +245,17 @@ public abstract static class ObjectToArrayPairNode extends PNodeWithContext {
252245
static ArrayBuilder<KeyValue> partialMerge(VirtualFrame frame, Object mapping, Object keyAttr,
253246
@Bind("this") Node inliningTarget,
254247
@Shared @Cached PyObjectGetIter getIter,
255-
@Shared @Cached GetNextNode nextNode,
248+
@Shared @Cached(neverDefault = false) PyIterNextNode nextNode,
256249
@Shared @Cached PyObjectGetItem getItemNode,
257-
@Shared @Cached IsBuiltinObjectProfile errorProfile,
258250
@Cached CallVarargsMethodNode callKeysMethod) {
259251
// We don't need to pass self as the attribute object has it already.
260252
Object keysIterable = callKeysMethod.execute(frame, keyAttr, EMPTY_OBJECT_ARRAY, EMPTY_KEYWORDS);
261253
Object keysIt = getIter.execute(frame, inliningTarget, keysIterable);
262254
ArrayBuilder<KeyValue> elements = new ArrayBuilder<>();
263-
while (true) {
264-
try {
265-
Object keyObj = nextNode.execute(frame, keysIt);
266-
Object valueObj = getItemNode.execute(frame, inliningTarget, mapping, keyObj);
267-
elements.add(new KeyValue(keyObj, valueObj));
268-
} catch (PException e) {
269-
e.expectStopIteration(inliningTarget, errorProfile);
270-
break;
271-
}
255+
Object keyObj;
256+
while ((keyObj = nextNode.execute(frame, keysIt)) != null) {
257+
Object valueObj = getItemNode.execute(frame, inliningTarget, mapping, keyObj);
258+
elements.add(new KeyValue(keyObj, valueObj));
272259
}
273260
return elements;
274261
}
@@ -278,19 +265,17 @@ static ArrayBuilder<KeyValue> partialMerge(VirtualFrame frame, Object mapping, O
278265
static ArrayBuilder<KeyValue> partialMergeFromSeq2(VirtualFrame frame, Object iterable, @SuppressWarnings("unused") PNone keyAttr,
279266
@Bind("this") Node inliningTarget,
280267
@Shared @Cached PyObjectGetIter getIter,
281-
@Shared @Cached GetNextNode nextNode,
268+
@Shared @Cached(neverDefault = false) PyIterNextNode nextNode,
282269
@Shared @Cached PyObjectGetItem getItemNode,
283-
@Shared @Cached IsBuiltinObjectProfile errorProfile,
284270
@Cached FastConstructListNode createListNode,
285271
@Cached LenNode seqLenNode,
286272
@Cached PRaiseNode.Lazy raise,
287-
@Cached InlinedConditionProfile lengthTwoProfile,
288-
@Exclusive @Cached IsBuiltinObjectProfile isTypeErrorProfile) throws PException {
273+
@Cached InlinedConditionProfile lengthTwoProfile) throws PException {
289274
Object it = getIter.execute(frame, inliningTarget, iterable);
290275
ArrayBuilder<KeyValue> elements = new ArrayBuilder<>();
276+
Object next;
291277
try {
292-
while (true) {
293-
Object next = nextNode.execute(frame, it);
278+
while ((next = nextNode.execute(frame, it)) != null) {
294279
PSequence element = createListNode.execute(frame, inliningTarget, next);
295280
assert element != null;
296281
// This constructs a new list using the builtin type. So, the object cannot
@@ -305,11 +290,7 @@ static ArrayBuilder<KeyValue> partialMergeFromSeq2(VirtualFrame frame, Object it
305290
elements.add(new KeyValue(key, value));
306291
}
307292
} catch (PException e) {
308-
if (isTypeErrorProfile.profileException(inliningTarget, e, TypeError)) {
309-
throw raise.get(inliningTarget).raise(TypeError, ErrorMessages.CANNOT_CONVERT_DICT_UPDATE_SEQ, elements.size());
310-
} else {
311-
e.expectStopIteration(inliningTarget, errorProfile);
312-
}
293+
throw raise.get(inliningTarget).raise(TypeError, ErrorMessages.CANNOT_CONVERT_DICT_UPDATE_SEQ, elements.size());
313294
}
314295
return elements;
315296
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/DictNodes.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,12 @@ static void updateDictGeneric(VirtualFrame frame, PDict self, PDict other,
114114
@Specialization(guards = "!isDict(other)")
115115
static void updateArg(VirtualFrame frame, PDict self, Object other,
116116
@Bind("this") Node inliningTarget,
117-
@Cached HashingStorageSetItem setHasihngStorageItem,
117+
@Cached HashingStorageSetItem setItem,
118118
@Cached PyObjectLookupAttr lookupKeys,
119119
@Cached ObjectToArrayPairNode toArrayPair) {
120120
Object keyAttr = lookupKeys.execute(frame, inliningTarget, other, T_KEYS);
121121
HashingStorage storage = addKeyValuesToStorage(frame, self, other, keyAttr,
122-
inliningTarget, toArrayPair, setHasihngStorageItem);
122+
inliningTarget, toArrayPair, setItem);
123123
self.setDictStorage(storage);
124124
}
125125

0 commit comments

Comments
 (0)