Skip to content

Commit 2000d28

Browse files
steve-ssteve-s
committed
ObjectHashMap: move put into a node
1 parent 2b54080 commit 2000d28

File tree

4 files changed

+36
-82
lines changed

4 files changed

+36
-82
lines changed

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import java.util.stream.Collectors;
5656
import java.util.stream.StreamSupport;
5757

58+
import com.oracle.truffle.api.profiles.BranchProfile;
5859
import com.oracle.truffle.api.profiles.ConditionProfile;
5960
import org.junit.Assert;
6061
import org.junit.Test;
@@ -104,7 +105,7 @@ public void testCollisionsByPuttingManyKeysWithSameHash() {
104105
DictKey key = new DictKey(42);
105106
Object value = newValue();
106107
expected.put(key, value);
107-
map.put(null, key, 42, value, PUT_PROFILES);
108+
put(map, key, 42, value);
108109
assertEqual(i, expected, map);
109110
}
110111
for (int i = 0; i < 55; i++) {
@@ -117,7 +118,7 @@ public void testCollisionsByPuttingManyKeysWithSameHash() {
117118
DictKey key = expected.keySet().stream().skip(10 + i).findFirst().get();
118119
Object value = newValue();
119120
expected.put(key, value);
120-
map.put(null, key, 42, value, PUT_PROFILES);
121+
put(map, key, 42, value);
121122
assertEqual(i, expected, map);
122123
}
123124
}
@@ -129,7 +130,7 @@ public void testCollisionsByPuttingAndRemovingTheSameKey() {
129130
DictKey key = new DictKey(42);
130131
for (int i = 0; i < 100; i++) {
131132
Object value = newValue();
132-
map.put(null, key, 42, value, PUT_PROFILES);
133+
put(map, key, 42, value);
133134
expected.put(key, value);
134135
assertEqual(i, expected, map);
135136

@@ -147,7 +148,7 @@ public void testCollisionsByPuttingAndRemovingTheSameKeys() {
147148
for (int i = 0; i < 100; i++) {
148149
Object value = newValue();
149150
final DictKey toPut = keys[i % keys.length];
150-
map.put(null, toPut, toPut.hash, value, PUT_PROFILES);
151+
put(map, toPut, toPut.hash, value);
151152
expected.put(toPut, value);
152153
assertEqual(i, expected, map);
153154

@@ -242,7 +243,7 @@ private static void overrideValues(ObjectHashMap map, LinkedHashMap<Long, Object
242243
Object value = newValue();
243244
int index = rand.nextInt(expected.size() - 1);
244245
long key = expected.keySet().stream().skip(index).findFirst().get();
245-
map.put(null, key, PyObjectHashNode.hash(key), value, PUT_PROFILES);
246+
put(map, key, PyObjectHashNode.hash(key), value);
246247
expected.put(key, value);
247248
assertEqual(i, expected, map);
248249
}
@@ -252,7 +253,7 @@ private static void putValues(ObjectHashMap map, LinkedHashMap<Long, Object> exp
252253
for (int i = 0; i < count; i++) {
253254
Object value = newValue();
254255
long key = rand.nextLong();
255-
map.put(null, key, PyObjectHashNode.hash(key), value, PUT_PROFILES);
256+
put(map, key, PyObjectHashNode.hash(key), value);
256257
expected.put(key, value);
257258
assertEqual(i, expected, map);
258259
}
@@ -325,4 +326,11 @@ private static Object get(ObjectHashMap map, Object key, long hash) {
325326
ConditionProfile.getUncached(), ConditionProfile.getUncached(), ConditionProfile.getUncached(),//
326327
new EqNodeStub());
327328
}
329+
330+
private static void put(ObjectHashMap map, Object key, long hash, Object value) {
331+
ObjectHashMap.PutNode.doPut(null, map, key, hash, value,//
332+
ConditionProfile.getUncached(), ConditionProfile.getUncached(), ConditionProfile.getUncached(),//
333+
ConditionProfile.getUncached(), BranchProfile.getUncached(), BranchProfile.getUncached(),//
334+
ConditionProfile.getUncached(), new EqNodeStub());
335+
}
328336
}

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
4949
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.DictKey;
5050
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.MapCursor;
51-
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutProfiles;
51+
import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutNode;
5252
import com.oracle.graal.python.builtins.objects.function.PArguments;
5353
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
5454
import com.oracle.graal.python.builtins.objects.object.PythonObject;
@@ -71,7 +71,6 @@
7171
import com.oracle.truffle.api.library.ExportLibrary;
7272
import com.oracle.truffle.api.library.ExportMessage;
7373
import com.oracle.truffle.api.nodes.LoopNode;
74-
import com.oracle.truffle.api.nodes.Node;
7574
import com.oracle.truffle.api.profiles.ConditionProfile;
7675
import com.oracle.truffle.api.profiles.LoopConditionProfile;
7776
import com.oracle.truffle.api.strings.TruffleString;
@@ -163,7 +162,7 @@ static boolean maySideEffect(PythonObject o, LookupInheritedAttributeNode.Dynami
163162
@Specialization
164163
static HashingStorage setItemTruffleString(EconomicMapStorage self, TruffleString key, Object value, ThreadState state,
165164
@Shared("tsHash") @Cached TruffleString.HashCodeNode hashCodeNode,
166-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
165+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
167166
@Shared("gotState") @Cached ConditionProfile gotState) {
168167
VirtualFrame frame = gotState.profile(state == null) ? null : PArguments.frameForCall(state);
169168
putNode.put(state, self.map, key, PyObjectHashNode.hash(key, hashCodeNode), assertNoJavaString(value));
@@ -174,7 +173,7 @@ static HashingStorage setItemTruffleString(EconomicMapStorage self, TruffleStrin
174173
static HashingStorage setItemPString(EconomicMapStorage self, PString key, Object value, ThreadState state,
175174
@Shared("stringMaterialize") @Cached StringMaterializeNode stringMaterializeNode,
176175
@Shared("tsHash") @Cached TruffleString.HashCodeNode hashCodeNode,
177-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
176+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
178177
@Shared("gotState") @Cached ConditionProfile gotState,
179178
@Shared("builtinProfile") @Cached IsBuiltinClassProfile isBuiltinClassProfile) {
180179
final TruffleString k = stringMaterializeNode.execute(key);
@@ -184,7 +183,7 @@ static HashingStorage setItemPString(EconomicMapStorage self, PString key, Objec
184183
@Specialization(guards = {"!hasSideEffect(self)", "!isBuiltin(key,builtinProfile) || !isBuiltin(value,builtinProfile)",
185184
"maySideEffect(key, lookup) || maySideEffect(value, lookup)"}, limit = "1")
186185
static HashingStorage setItemPythonObjectWithSideEffect(EconomicMapStorage self, PythonObject key, PythonObject value, ThreadState state,
187-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
186+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
188187
@Shared("hashNode") @Cached PyObjectHashNode hashNode,
189188
@Shared("lookup") @Cached LookupInheritedAttributeNode.Dynamic lookup,
190189
@Shared("builtinProfile") @Cached IsBuiltinClassProfile builtinProfile,
@@ -195,7 +194,7 @@ static HashingStorage setItemPythonObjectWithSideEffect(EconomicMapStorage self,
195194

196195
@Specialization(guards = {"!hasSideEffect(self)", "!isBuiltin(key,builtinProfile)", "maySideEffect(key, lookup)"}, limit = "1")
197196
static HashingStorage setItemPythonObjectWithSideEffect(EconomicMapStorage self, PythonObject key, Object value, ThreadState state,
198-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
197+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
199198
@Shared("hashNode") @Cached PyObjectHashNode hashNode,
200199
@Shared("lookup") @Cached LookupInheritedAttributeNode.Dynamic lookup,
201200
@Shared("builtinProfile") @Cached IsBuiltinClassProfile builtinProfile,
@@ -206,7 +205,7 @@ static HashingStorage setItemPythonObjectWithSideEffect(EconomicMapStorage self,
206205

207206
@Specialization(guards = {"!hasSideEffect(self)", "!isBuiltin(value,builtinProfile)", "maySideEffect(value, lookup)"}, limit = "1")
208207
static HashingStorage setItemPythonObjectWithSideEffect(EconomicMapStorage self, Object key, PythonObject value, ThreadState state,
209-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
208+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
210209
@Shared("hashNode") @Cached PyObjectHashNode hashNode,
211210
@Shared("lookup") @Cached LookupInheritedAttributeNode.Dynamic lookup,
212211
@Shared("builtinProfile") @Cached IsBuiltinClassProfile builtinProfile,
@@ -217,7 +216,7 @@ static HashingStorage setItemPythonObjectWithSideEffect(EconomicMapStorage self,
217216

218217
@Specialization(replaces = {"setItemPString", "setItemTruffleString"})
219218
static HashingStorage setItemGeneric(EconomicMapStorage self, Object key, Object value, ThreadState state,
220-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
219+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
221220
@Shared("hashNode") @Cached PyObjectHashNode hashNode,
222221
@Shared("gotState") @Cached ConditionProfile gotState) {
223222
VirtualFrame frame = gotState.profile(state == null) ? null : PArguments.frameForCall(state);
@@ -260,23 +259,23 @@ protected static boolean hasSideEffect(EconomicMapStorage self, EconomicMapStora
260259
static HashingStorage toSameTypeSideEffect(EconomicMapStorage self, EconomicMapStorage other,
261260
@CachedLibrary("self") HashingStorageLibrary thisLib,
262261
@Shared("selfEntriesLoop") @Cached LoopConditionProfile loopProfile,
263-
@Shared("putProfiles") @Cached ObjectHashMap.PutProfiles profiles) {
262+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode) {
264263
convertToSideEffectMap(other);
265-
return toSameType(self, other, thisLib, loopProfile, profiles);
264+
return toSameType(self, other, thisLib, loopProfile, putNode);
266265
}
267266

268267
@Specialization(guards = "!hasSideEffect(self, other)")
269268
static HashingStorage toSameType(EconomicMapStorage self, EconomicMapStorage other,
270269
@CachedLibrary("self") HashingStorageLibrary thisLib,
271270
@Shared("selfEntriesLoop") @Cached LoopConditionProfile loopProfile,
272-
@Shared("putProfiles") @Cached ObjectHashMap.PutProfiles profiles) {
271+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode) {
273272
MapCursor cursor = self.map.getEntries();
274273
// get/put may throw, but we ignore that small inaccuracy
275274
final int size = self.map.size();
276275
loopProfile.profileCounted(size);
277276
LoopNode.reportLoopCount(thisLib, size);
278277
while (loopProfile.inject(advance(cursor))) {
279-
other.map.put(null, cursor.getKey().getValue(), cursor.getKey().getPythonHash(), cursor.getValue(), profiles);
278+
putNode.put(null, other.map, cursor.getKey().getValue(), cursor.getKey().getPythonHash(), cursor.getValue());
280279
}
281280
return other;
282281
}
@@ -522,7 +521,7 @@ public static class DiffWithState {
522521
@Specialization
523522
static HashingStorage diffSameType(EconomicMapStorage self, EconomicMapStorage other, ThreadState state,
524523
@CachedLibrary("self") HashingStorageLibrary thisLib,
525-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
524+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
526525
@Shared("getNode") @Cached ObjectHashMap.GetNode getNode,
527526
@Shared("selfEntriesLoop") @Cached LoopConditionProfile loopProfile,
528527
@Shared("gotState") @Cached ConditionProfile gotState) {
@@ -544,7 +543,7 @@ static HashingStorage diffSameType(EconomicMapStorage self, EconomicMapStorage o
544543
@Specialization
545544
static HashingStorage diffGeneric(EconomicMapStorage self, HashingStorage other, @SuppressWarnings("unused") ThreadState state,
546545
@CachedLibrary("self") HashingStorageLibrary thisLib,
547-
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
546+
@Shared("putNode") @Cached ObjectHashMap.PutNode putNode,
548547
@Shared("selfEntriesLoop") @Cached LoopConditionProfile loopProfile,
549548
@Shared("otherHLib") @CachedLibrary(limit = "2") HashingStorageLibrary hlib,
550549
@Shared("gotState") @Cached ConditionProfile gotState) {
@@ -585,19 +584,20 @@ public HashingStorageIterable<Object> reverseKeys() {
585584
return map.reverseKeys();
586585
}
587586

588-
protected void setValueForAllKeys(VirtualFrame frame, Object value, ObjectHashMap.PutProfiles profiles, Node node, LoopConditionProfile loopProfile) {
587+
protected void setValueForAllKeys(VirtualFrame frame, Object value, PutNode putNode, ConditionProfile hasFrame, LoopConditionProfile loopProfile) {
589588
MapCursor cursor = map.getEntries();
590589
final int size = map.size();
591590
loopProfile.profileCounted(size);
592-
LoopNode.reportLoopCount(node, size);
591+
LoopNode.reportLoopCount(putNode, size);
592+
ThreadState state = PArguments.getThreadStateOrNull(frame, hasFrame);
593593
while (loopProfile.inject(advance(cursor))) {
594-
map.put(frame, getDictKey(cursor), value, profiles);
594+
putNode.put(state, map, getDictKey(cursor), value);
595595
}
596596
}
597597

598598
@TruffleBoundary
599599
public void putUncached(TruffleString key, Object value) {
600-
map.put(null, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value, PutProfiles.getUncached());
600+
ObjectHashMapFactory.PutNodeGen.getUncached().put(null, this.map, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
601601
}
602602

603603
@Override

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,11 @@ abstract static class SetValueHashingStorageNode extends PNodeWithContext {
122122

123123
@Specialization
124124
HashingStorage doEconomicStorage(VirtualFrame frame, EconomicMapStorage map, Object value,
125-
@Cached ObjectHashMap.PutProfiles profiles,
125+
@Cached ObjectHashMap.PutNode putNode,
126+
@Cached ConditionProfile hasFrameProfile,
126127
@Cached LoopConditionProfile loopProfile) {
127128
// We want to avoid calling __hash__() during map.put
128-
map.setValueForAllKeys(frame, value, profiles, this, loopProfile);
129+
map.setValueForAllKeys(frame, value, putNode, hasFrameProfile, loopProfile);
129130
return map;
130131
}
131132

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

Lines changed: 1 addition & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,7 @@ public final void put(ThreadState state, ObjectHashMap map, Object key, long key
587587

588588
abstract void execute(ThreadState state, ObjectHashMap map, Object key, long keyHash, Object value);
589589

590+
// "public" for testing...
590591
@Specialization
591592
public static void doPut(ThreadState state, ObjectHashMap map, Object key, long keyHash, Object value,
592593
@Cached("createCountingProfile()") ConditionProfile foundNullKey,
@@ -645,62 +646,6 @@ public static void doPut(ThreadState state, ObjectHashMap map, Object key, long
645646
}
646647
}
647648

648-
public void put(VirtualFrame frame, DictKey key, Object value, PutProfiles profiles) {
649-
assert checkInternalState();
650-
put(frame, key.getValue(), key.getPythonHash(), value, profiles, profiles.eqNode, true);
651-
}
652-
653-
public void put(VirtualFrame frame, Object key, long keyHash, Object value, PutProfiles profiles) {
654-
assert checkInternalState();
655-
put(frame, key, keyHash, value, profiles, profiles.eqNode, true);
656-
}
657-
658-
public void put(VirtualFrame frame, Object key, long keyHash, Object value, PutProfiles profiles, PyObjectRichCompareBool.EqNode eqNode, boolean canRehash) {
659-
CompilerAsserts.partialEvaluationConstant(canRehash);
660-
661-
int compactIndex = getIndex(keyHash);
662-
int index = indices[compactIndex];
663-
if (profiles.foundNullKey.profile(index == EMPTY_INDEX)) {
664-
putInNewSlot(profiles.rehash1Profile, key, keyHash, value, compactIndex);
665-
return;
666-
}
667-
668-
if (profiles.foundEqKey.profile(index != DUMMY_INDEX && keysEqual(frame, unwrapIndex(index), key, keyHash, eqNode))) {
669-
// we found the key, override the value, Python does not override the key though
670-
setValue(unwrapIndex(index), value);
671-
return;
672-
}
673-
674-
// collision
675-
markCollision(compactIndex);
676-
long perturb = keyHash;
677-
int searchLimit = getBucketsCount() + PERTURB_SHIFTS_COUT;
678-
int i = 0;
679-
try {
680-
for (; CompilerDirectives.injectBranchProbability(0, i < searchLimit); i++) {
681-
perturb >>>= PERTURB_SHIFT;
682-
compactIndex = nextIndex(compactIndex, perturb);
683-
index = indices[compactIndex];
684-
if (profiles.collisionFoundNoValue.profile(index == EMPTY_INDEX)) {
685-
putInNewSlot(profiles.rehash2Profile, key, keyHash, value, compactIndex);
686-
return;
687-
}
688-
if (profiles.collisionFoundEqKey.profile(index != DUMMY_INDEX && keysEqual(frame, unwrapIndex(index), key, keyHash, eqNode))) {
689-
// we found the key, override the value, Python does not override the key though
690-
setValue(unwrapIndex(index), value);
691-
return;
692-
}
693-
markCollision(compactIndex);
694-
}
695-
} finally {
696-
LoopNode.reportLoopCount(profiles, i);
697-
}
698-
// all values are dummies? Not possible, since we should have compacted the
699-
// hashes/keysAndValues arrays in "remove". Also, there must be an unused slot available,
700-
// because we checked at the beginning that we have at least ~1/4 of space left
701-
throw CompilerDirectives.shouldNotReachHere();
702-
}
703-
704649
// Internal helper: it is not profiling, never rehashes, and it assumes that the hash map never
705650
// contains the key that we are inserting
706651
private void insertNewKey(Object key, long keyHash, Object value) {

0 commit comments

Comments
 (0)