Skip to content

Commit 4942be1

Browse files
steve-ssteve-s
committed
Set/dict literals: preallocate hashing storage, remove unnecessary @ExplodeLoop in Bytecode DSL
1 parent afdc0b8 commit 4942be1

File tree

11 files changed

+303
-150
lines changed

11 files changed

+303
-150
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/objects/ObjectHashMapTests.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import java.util.Random;
5353
import java.util.stream.Collectors;
5454

55-
import com.oracle.graal.python.lib.RichCmpOp;
5655
import org.junit.Assert;
5756
import org.junit.Test;
5857

@@ -68,8 +67,10 @@
6867
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap;
6968
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.MapCursor;
7069
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PopNode;
70+
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutUnsafeNode;
7171
import com.oracle.graal.python.lib.PyObjectHashNode;
7272
import com.oracle.graal.python.lib.PyObjectRichCompareBool;
73+
import com.oracle.graal.python.lib.RichCmpOp;
7374
import com.oracle.truffle.api.frame.Frame;
7475
import com.oracle.truffle.api.interop.TruffleObject;
7576
import com.oracle.truffle.api.nodes.Node;
@@ -100,7 +101,7 @@ public boolean execute(Frame frame, Node inliningTarget, Object a, Object b, Ric
100101

101102
@Test
102103
public void testCollisionsByPuttingManyKeysWithSameHash() {
103-
ObjectHashMap map = new ObjectHashMap();
104+
ObjectHashMap map = new ObjectHashMap(true);
104105
LinkedHashMap<DictKey, Object> expected = new LinkedHashMap<>();
105106
for (int i = 0; i < 100; i++) {
106107
DictKey key = new DictKey(42);
@@ -126,7 +127,7 @@ public void testCollisionsByPuttingManyKeysWithSameHash() {
126127

127128
@Test
128129
public void testCollisionsByPuttingAndRemovingTheSameKey() {
129-
ObjectHashMap map = new ObjectHashMap();
130+
ObjectHashMap map = new ObjectHashMap(true);
130131
LinkedHashMap<DictKey, Object> expected = new LinkedHashMap<>();
131132
DictKey key = new DictKey(42);
132133
for (int i = 0; i < 100; i++) {
@@ -143,7 +144,7 @@ public void testCollisionsByPuttingAndRemovingTheSameKey() {
143144

144145
@Test
145146
public void testCollisionsByPuttingAndRemovingTheSameKeys() {
146-
ObjectHashMap map = new ObjectHashMap();
147+
ObjectHashMap map = new ObjectHashMap(true);
147148
LinkedHashMap<DictKey, Object> expected = new LinkedHashMap<>();
148149
DictKey[] keys = new DictKey[]{new DictKey(42), new DictKey(1)};
149150
for (int i = 0; i < 100; i++) {
@@ -162,7 +163,7 @@ public void testCollisionsByPuttingAndRemovingTheSameKeys() {
162163

163164
@Test
164165
public void testLongHashMapStressTest() {
165-
ObjectHashMap map = new ObjectHashMap();
166+
ObjectHashMap map = new ObjectHashMap(true);
166167

167168
// put/remove many random (with fixed seed) keys, check consistency against LinkedHashMap
168169
testBasics(map);
@@ -373,7 +374,7 @@ private static void remove(ObjectHashMap map, Object key, long hash) {
373374

374375
private static void put(ObjectHashMap map, Object key, long hash, Object value) {
375376
InlinedCountingConditionProfile uncachedCounting = InlinedCountingConditionProfile.getUncached();
376-
ObjectHashMap.PutNode.doPutWithRestart(null, null, map, key, hash, value,
377+
PutUnsafeNode.doPutWithRestart(null, null, map, key, hash, value,
377378
InlinedBranchProfile.getUncached(), uncachedCounting, uncachedCounting, uncachedCounting,
378379
uncachedCounting, InlinedBranchProfile.getUncached(), InlinedBranchProfile.getUncached(),
379380
new EqNodeStub());

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_pystate.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
1+
# Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
22
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
33
#
44
# The Universal Permissive License (UPL), Version 1.0
@@ -111,4 +111,4 @@ def other_thread():
111111
SetAsyncExcCaller.trigger_ex(t.ident, Exception("test my message"))
112112
t.join()
113113

114-
assert "test my message" in str(caught_ex)
114+
assert "test my message" in str(caught_ex), str(caught_ex)

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -50,6 +50,7 @@
5050
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.DictKey;
5151
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.MapCursor;
5252
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutNode;
53+
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutUnsafeNode;
5354
import com.oracle.graal.python.lib.PyObjectHashNode;
5455
import com.oracle.truffle.api.CompilerAsserts;
5556
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -131,11 +132,15 @@ void clear() {
131132
map.clear();
132133
}
133134

135+
public boolean mapIsEqualTo(ObjectHashMap other) {
136+
return other == this.map;
137+
}
138+
134139
public HashingStorage copy() {
135140
return new EconomicMapStorage(this.map, true);
136141
}
137142

138-
protected void setValueForAllKeys(VirtualFrame frame, Node inliningTarget, Object value, PutNode putNode, InlinedLoopConditionProfile loopProfile) {
143+
protected void setValueForAllKeys(VirtualFrame frame, Node inliningTarget, Object value, PutUnsafeNode putNode, InlinedLoopConditionProfile loopProfile) {
139144
MapCursor cursor = map.getEntries();
140145
final int size = map.size();
141146
loopProfile.profileCounted(inliningTarget, size);
@@ -147,17 +152,17 @@ protected void setValueForAllKeys(VirtualFrame frame, Node inliningTarget, Objec
147152

148153
@TruffleBoundary
149154
public void putUncached(TruffleString key, Object value) {
150-
ObjectHashMap.PutNode.putUncached(this.map, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
155+
PutUnsafeNode.putUncached(this.map, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
151156
}
152157

153158
private void putUncached(Object key, Object value) {
154-
ObjectHashMap.PutNode.putUncached(this.map, key, PyObjectHashNode.executeUncached(key), value);
159+
PutNode.getUncached().put(null, null, this.map, key, PyObjectHashNode.executeUncached(key), value);
155160
}
156161

157162
// Solves boot-order problem, do not use in normal code or during startup when __eq__ of
158163
// builtins may not be properly set-up
159164
public void putUncachedWithJavaEq(Object key, long keyHash, Object value) {
160-
ObjectHashMap.PutNode.putUncachedWithJavaEq(this.map, key, keyHash, value);
165+
PutUnsafeNode.putUncachedWithJavaEq(this.map, key, keyHash, value);
161166
}
162167

163168
public void putUncachedWithJavaEq(TruffleString key, Object value) {
@@ -212,7 +217,7 @@ public abstract static class EconomicMapSetStringKey extends SpecializedSetStrin
212217
@Specialization
213218
static void doIt(Node inliningTarget, HashingStorage self, TruffleString key, Object value,
214219
@Cached PyObjectHashNode hashNode,
215-
@Cached PutNode putNode) {
220+
@Cached PutUnsafeNode putNode) {
216221
putNode.put(null, inliningTarget, ((EconomicMapStorage) self).map, key, hashNode.execute(null, inliningTarget, key), value);
217222
}
218223
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorKey;
5252
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorNext;
5353
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageSetItem;
54+
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutUnsafeNode;
5455
import com.oracle.graal.python.builtins.objects.dict.DictNodes;
5556
import com.oracle.graal.python.builtins.objects.dict.PDict;
5657
import com.oracle.graal.python.builtins.objects.dict.PDictView;
@@ -120,7 +121,7 @@ abstract static class SetValueHashingStorageNode extends PNodeWithContext {
120121

121122
@Specialization
122123
static HashingStorage doEconomicStorage(VirtualFrame frame, Node inliningTarget, EconomicMapStorage map, Object value,
123-
@Cached ObjectHashMap.PutNode putNode,
124+
@Cached PutUnsafeNode putNode,
124125
@Cached InlinedLoopConditionProfile loopProfile) {
125126
// We want to avoid calling __hash__() during map.put
126127
map.setValueForAllKeys(frame, inliningTarget, value, putNode, loopProfile);

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

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageSetItemWithHashNodeGen;
6363
import com.oracle.graal.python.builtins.objects.common.KeywordsStorage.GetKeywordsStorageItemNode;
6464
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutNode;
65+
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutUnsafeNode;
6566
import com.oracle.graal.python.lib.PyObjectHashNode;
6667
import com.oracle.graal.python.lib.PyObjectRichCompareBool;
6768
import com.oracle.graal.python.lib.PyUnicodeCheckExactNode;
@@ -121,7 +122,7 @@ public static boolean mayHaveSideEffectingEq(HashingStorage storage) {
121122

122123
public static boolean mayHaveSideEffects(PHashingCollection wrapper) {
123124
HashingStorage s = wrapper.getDictStorage();
124-
return !(s instanceof EconomicMapStorage && ((EconomicMapStorage) s).map.hasSideEffect());
125+
return !(s instanceof EconomicMapStorage && ((EconomicMapStorage) s).map.hasSideEffectingKeys());
125126
}
126127
}
127128

@@ -252,7 +253,7 @@ abstract static class SpecializedSetStringKey extends Node {
252253
public abstract void execute(Node inliningTarget, HashingStorage self, TruffleString key, Object value);
253254
}
254255

255-
static EconomicMapStorage dynamicObjectStorageToEconomicMap(Node inliningTarget, DynamicObjectStorage s, DynamicObjectLibrary dylib, PyObjectHashNode hashNode, PutNode putNode) {
256+
static EconomicMapStorage dynamicObjectStorageToEconomicMap(Node inliningTarget, DynamicObjectStorage s, DynamicObjectLibrary dylib, PyObjectHashNode hashNode, PutUnsafeNode putNode) {
256257
// TODO: shouldn't we invalidate all MRO assumptions in this case?
257258
DynamicObject store = s.store;
258259
EconomicMapStorage result = EconomicMapStorage.create(dylib.getShape(store).getPropertyCount());
@@ -287,24 +288,16 @@ public final HashingStorage executeCached(Frame frame, HashingStorage self, Obje
287288

288289
@Specialization
289290
static HashingStorage economicMap(Frame frame, Node inliningTarget, EconomicMapStorage self, Object key, long keyHash, Object value,
290-
@Exclusive @Cached PyUnicodeCheckExactNode isBuiltinString,
291-
@Exclusive @Cached ObjectHashMap.PutNode putNode) {
291+
@Exclusive @Cached PutNode putNode) {
292292
putNode.execute(frame, inliningTarget, self.map, key, keyHash, value);
293-
if (!self.map.hasSideEffect() && !isBuiltinString.execute(inliningTarget, key)) {
294-
self.map.setSideEffectingKeysFlag();
295-
}
296293
return self;
297294
}
298295

299296
@Specialization
300297
static HashingStorage empty(Frame frame, Node inliningTarget, @SuppressWarnings("unused") EmptyStorage self, Object key, long keyHash, Object value,
301-
@Exclusive @Cached PyUnicodeCheckExactNode isBuiltinString,
302-
@Exclusive @Cached ObjectHashMap.PutNode putNode) {
298+
@Exclusive @Cached PutNode putNode) {
303299
EconomicMapStorage storage = EconomicMapStorage.create(1);
304300
putNode.execute(frame, inliningTarget, storage.map, key, keyHash, value);
305-
if (!isBuiltinString.execute(inliningTarget, key)) {
306-
storage.map.setSideEffectingKeysFlag();
307-
}
308301
return storage;
309302
}
310303

@@ -335,17 +328,16 @@ static HashingStorage dom(Frame frame, Node inliningTarget, DynamicObjectStorage
335328
@Specialization
336329
@InliningCutoff
337330
static HashingStorage keywords(Frame frame, Node inliningTarget, KeywordsStorage self, Object key, long keyHash, Object value,
338-
@Exclusive @Cached PyUnicodeCheckExactNode isBuiltinString,
339-
@Exclusive @Cached ObjectHashMap.PutNode putNode,
331+
@Exclusive @Cached PutNode putNode,
340332
@Cached EconomicMapSetStringKey specializedPutNode) {
341333
// TODO: do we want to try DynamicObjectStorage if the key is a string?
342334
EconomicMapStorage result = EconomicMapStorage.create(self.length());
343335
self.addAllTo(inliningTarget, result, specializedPutNode);
344-
return economicMap(frame, inliningTarget, result, key, keyHash, value, isBuiltinString, putNode);
336+
return economicMap(frame, inliningTarget, result, key, keyHash, value, putNode);
345337
}
346338

347339
@Specialization
348-
static HashingStorage foreign(Frame frame, Node inliningTarget, ForeignHashingStorage self, Object key, long keyHash, Object value,
340+
static HashingStorage foreign(Node inliningTarget, ForeignHashingStorage self, Object key, long keyHash, Object value,
349341
@Cached ForeignHashingStorage.PutNode putNode) {
350342
putNode.execute(inliningTarget, self, key, value);
351343
return self;
@@ -373,8 +365,9 @@ static HashingStorage domStringKey(Node inliningTarget, DynamicObjectStorage sel
373365
static HashingStorage domTransition(Frame frame, Node inliningTarget, DynamicObjectStorage self, Object key, @SuppressWarnings("unused") long keyHash, Object value,
374366
@SuppressWarnings("unused") boolean transition, DynamicObjectLibrary dylib,
375367
@Cached PyObjectHashNode hashNode,
376-
@Cached ObjectHashMap.PutNode putNode) {
377-
EconomicMapStorage result = dynamicObjectStorageToEconomicMap(inliningTarget, self, dylib, hashNode, putNode);
368+
@Cached PutUnsafeNode putUnsafeNode,
369+
@Cached PutNode putNode) {
370+
EconomicMapStorage result = dynamicObjectStorageToEconomicMap(inliningTarget, self, dylib, hashNode, putUnsafeNode);
378371
putNode.execute(frame, inliningTarget, result.map, key, keyHash, value);
379372
return result;
380373
}
@@ -415,27 +408,22 @@ public final HashingStorage execute(Node inliningTarget, HashingStorage self, Tr
415408

416409
@Specialization
417410
static HashingStorage economicMap(Frame frame, Node inliningTarget, EconomicMapStorage self, Object key, Object value,
418-
@Exclusive @Cached PyUnicodeCheckExactNode isBuiltinString,
419411
@Exclusive @Cached PyObjectHashNode hashNode,
420-
@Exclusive @Cached ObjectHashMap.PutNode putNode) {
412+
@Exclusive @Cached PutNode putNode) {
421413
putNode.execute(frame, inliningTarget, self.map, key, hashNode.execute(frame, inliningTarget, key), value);
422-
if (!self.map.hasSideEffect() && !isBuiltinString.execute(inliningTarget, key)) {
423-
self.map.setSideEffectingKeysFlag();
424-
}
425414
return self;
426415
}
427416

428417
@Specialization
429418
static HashingStorage empty(Frame frame, Node inliningTarget, @SuppressWarnings("unused") EmptyStorage self, Object key, Object value,
430-
@Exclusive @Cached PyUnicodeCheckExactNode isBuiltinString,
431419
@Exclusive @Cached PyObjectHashNode hashNode,
432-
@Exclusive @Cached ObjectHashMap.PutNode putNode) {
420+
@Exclusive @Cached PutNode putNode) {
433421
// The ObjectHashMap.PutNode is @Exclusive because profiles for a put into a freshly new
434422
// allocated map can be quite different to profiles in the other situations when we are
435423
// putting into a map that already has or will have some more items in it
436424
// It is also @Cached(inline = false) because inlining it triggers GR-44836
437425
// TODO: do we want to try DynamicObjectStorage if the key is a string?
438-
return economicMap(frame, inliningTarget, EconomicMapStorage.create(1), key, value, isBuiltinString, hashNode, putNode);
426+
return economicMap(frame, inliningTarget, EconomicMapStorage.create(1), key, value, hashNode, putNode);
439427
}
440428

441429
@Specialization(guards = "!self.shouldTransitionOnPut()")
@@ -466,13 +454,12 @@ static HashingStorage dom(Frame frame, Node inliningTarget, DynamicObjectStorage
466454
@InliningCutoff
467455
static HashingStorage keywords(Frame frame, Node inliningTarget, KeywordsStorage self, Object key, Object value,
468456
@Exclusive @Cached PyObjectHashNode hashNode,
469-
@Exclusive @Cached PyUnicodeCheckExactNode isBuiltinString,
470-
@Exclusive @Cached ObjectHashMap.PutNode putNode,
457+
@Exclusive @Cached PutNode putNode,
471458
@Cached EconomicMapSetStringKey specializedPutNode) {
472459
// TODO: do we want to try DynamicObjectStorage if the key is a string?
473460
EconomicMapStorage result = EconomicMapStorage.create(self.length());
474461
self.addAllTo(inliningTarget, result, specializedPutNode);
475-
return economicMap(frame, inliningTarget, result, key, value, isBuiltinString, hashNode, putNode);
462+
return economicMap(frame, inliningTarget, result, key, value, hashNode, putNode);
476463
}
477464

478465
@Specialization
@@ -504,8 +491,9 @@ static HashingStorage domStringKey(Node inliningTarget, DynamicObjectStorage sel
504491
static HashingStorage domTransition(Frame frame, Node inliningTarget, DynamicObjectStorage self, Object key, Object value,
505492
@SuppressWarnings("unused") boolean transition, DynamicObjectLibrary dylib,
506493
@Cached PyObjectHashNode hashNode,
507-
@Cached ObjectHashMap.PutNode putNode) {
508-
EconomicMapStorage result = dynamicObjectStorageToEconomicMap(inliningTarget, self, dylib, hashNode, putNode);
494+
@Cached PutUnsafeNode putUnsafeNode,
495+
@Cached PutNode putNode) {
496+
EconomicMapStorage result = dynamicObjectStorageToEconomicMap(inliningTarget, self, dylib, hashNode, putUnsafeNode);
509497
putNode.execute(frame, inliningTarget, result.map, key, hashNode.execute(frame, inliningTarget, key), value);
510498
return result;
511499
}
@@ -1365,7 +1353,7 @@ public abstract static class HashingStorageXorCallback extends HashingStorageFor
13651353

13661354
@Specialization
13671355
static ResultAndOther doGeneric(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ResultAndOther acc,
1368-
@Cached ObjectHashMap.PutNode putResultNode,
1356+
@Cached PutNode putResultNode,
13691357
@Cached HashingStorageGetItemWithHash getFromOther,
13701358
@Cached HashingStorageIteratorKey iterKey,
13711359
@Cached HashingStorageIteratorValue iterValue,
@@ -1417,7 +1405,7 @@ public abstract static class HashingStorageIntersectCallback extends HashingStor
14171405

14181406
@Specialization
14191407
static ResultAndOther doGeneric(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ResultAndOther acc,
1420-
@Cached ObjectHashMap.PutNode putResultNode,
1408+
@Cached PutNode putResultNode,
14211409
@Cached HashingStorageGetItemWithHash getFromOther,
14221410
@Cached HashingStorageIteratorKey iterKey,
14231411
@Cached HashingStorageIteratorKeyHash iterHash) {
@@ -1463,7 +1451,7 @@ public abstract static class HashingStorageDiffCallback extends HashingStorageFo
14631451

14641452
@Specialization
14651453
static ResultAndOther doGeneric(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ResultAndOther acc,
1466-
@Cached ObjectHashMap.PutNode putResultNode,
1454+
@Cached PutNode putResultNode,
14671455
@Cached HashingStorageGetItemWithHash getFromOther,
14681456
@Cached HashingStorageIteratorKey iterKey,
14691457
@Cached HashingStorageIteratorKeyHash iterHash,

0 commit comments

Comments
 (0)