Skip to content

Commit bdf82de

Browse files
timfelqunaibit
timfel
authored and
qunaibit
committed
Make iterating over hashes more convenient
This benefits dict.__repr__ and dictviews. We now have a generic forEach message to implement, and injectInto is a convenience method in terms of it. The HashingStorageLibrary now also returns a final subclass of Iterable, and its iterator method a final subclass of Iterator. This final subclass has boundaries on the next and hasNext calls, so we avoid the interface calls during PE, and the implementations of HashingStorageLibrary don't have to provide those boundaries. We also now get the iterator in the node call site when creating a dict view.
1 parent 55bed30 commit bdf82de

22 files changed

+295
-187
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171

7272
import java.math.BigInteger;
7373
import java.util.Arrays;
74-
import java.util.Iterator;
7574
import java.util.List;
7675
import java.util.Locale;
7776
import java.util.function.Supplier;
@@ -2318,8 +2317,7 @@ Object type(VirtualFrame frame, LazyPythonClass cls, String name, PTuple bases,
23182317
try {
23192318
PythonClass newType = typeMetaclass(frame, name, bases, namespace, metaclass, nslib);
23202319

2321-
for (Iterator<DictEntry> it = nslib.entries(namespace.getDictStorage()); it.hasNext();) {
2322-
DictEntry entry = it.next();
2320+
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
23232321
Object setName = getSetNameNode.execute(entry.value);
23242322
if (setName != PNone.NO_VALUE) {
23252323
callSetNameNode.execute(frame, setName, entry.value, newType, entry.key);
@@ -2410,7 +2408,7 @@ private PythonClass typeMetaclass(VirtualFrame frame, String name, PTuple bases,
24102408
// copy the dictionary slots over, as CPython does through PyDict_Copy
24112409
// Also check for a __slots__ sequence variable in dict
24122410
Object slots = null;
2413-
for (DictEntry entry : namespace.entries()) {
2411+
for (DictEntry entry : nslib.entries(namespace.getDictStorage())) {
24142412
Object key = entry.getKey();
24152413
Object value = entry.getValue();
24162414
if (__SLOTS__.equals(key)) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/DynamicObjectNativeWrapper.java

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import static com.oracle.graal.python.nodes.SpecialMethodNames.__SETATTR__;
6464
import static com.oracle.graal.python.nodes.SpecialMethodNames.__STR__;
6565

66+
import java.util.Set;
6667
import java.util.logging.Level;
6768

6869
import com.oracle.graal.python.PythonLanguage;
@@ -136,6 +137,7 @@
136137
import com.oracle.truffle.api.CompilerAsserts;
137138
import com.oracle.truffle.api.CompilerDirectives;
138139
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
140+
import com.oracle.truffle.api.CompilerDirectives.ValueType;
139141
import com.oracle.truffle.api.TruffleLogger;
140142
import com.oracle.truffle.api.dsl.Cached;
141143
import com.oracle.truffle.api.dsl.Cached.Exclusive;
@@ -941,15 +943,53 @@ Object doTpAlloc(PythonAbstractClass object, @SuppressWarnings("unused") String
941943
return allocFunc;
942944
}
943945

946+
@ValueType
947+
private static final class SubclassAddState {
948+
private final HashingStorage storage;
949+
private final HashingStorageLibrary lib;
950+
private final Set<PythonAbstractClass> subclasses;
951+
952+
private SubclassAddState(HashingStorage storage, HashingStorageLibrary lib, Set<PythonAbstractClass> subclasses) {
953+
this.storage = storage;
954+
this.lib = lib;
955+
this.subclasses = subclasses;
956+
}
957+
}
958+
959+
static final class EachSubclassAdd extends HashingStorageLibrary.ForEachNode<SubclassAddState> {
960+
private static final EachSubclassAdd SINGLETON = new EachSubclassAdd();
961+
962+
@Override
963+
public SubclassAddState execute(Object key, SubclassAddState s) {
964+
setAdd(s.subclasses, (PythonClass) s.lib.getItem(s.storage, key));
965+
return s;
966+
}
967+
968+
@TruffleBoundary
969+
protected static void setAdd(Set<PythonAbstractClass> set, PythonClass cls) {
970+
set.add(cls);
971+
}
972+
973+
static EachSubclassAdd create() {
974+
return SINGLETON;
975+
}
976+
977+
static EachSubclassAdd getUncached() {
978+
return SINGLETON;
979+
}
980+
}
981+
944982
@Specialization(guards = "eq(TP_SUBCLASSES, key)", limit = "1")
945-
@TruffleBoundary
946983
Object doTpSubclasses(PythonClass object, @SuppressWarnings("unused") String key, DynamicObjectNativeWrapper.PythonObjectNativeWrapper value,
947-
@CachedLibrary("value") PythonNativeWrapperLibrary lib) {
948-
// TODO more type checking; do fast path
984+
@Cached GetSubclassesNode getSubclassesNode,
985+
@Cached EachSubclassAdd eachNode,
986+
@Cached HashingCollectionNodes.GetDictStorageNode getStorage,
987+
@CachedLibrary("value") PythonNativeWrapperLibrary lib,
988+
@CachedLibrary(limit = "1") HashingStorageLibrary hashLib) {
949989
PDict dict = (PDict) lib.getDelegate(value);
950-
for (Object v : dict.items()) {
951-
GetSubclassesNode.doSlowPath(object).add((PythonClass) v);
952-
}
990+
HashingStorage storage = getStorage.execute(dict);
991+
Set<PythonAbstractClass> subclasses = getSubclassesNode.execute(object);
992+
hashLib.forEach(storage, eachNode, new SubclassAddState(storage, hashLib, subclasses));
953993
return value;
954994
}
955995

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

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
import java.util.NoSuchElementException;
4545

4646
import com.oracle.graal.python.builtins.objects.PNone;
47-
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.InjectIntoNode;
47+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.ForEachNode;
48+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
4849
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
4950
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
5051
import com.oracle.graal.python.builtins.objects.str.PString;
@@ -300,32 +301,32 @@ public HashingStorage delItemWithState(Object key, ThreadState state,
300301
}
301302

302303
@ExportMessage
303-
static class InjectInto {
304+
static class ForEachUntyped {
304305
@Specialization(guards = "cachedShape == self.store.getShape()", limit = "1")
305306
@ExplodeLoop
306-
static HashingStorage[] cachedLen(DynamicObjectStorage self, HashingStorage[] firstValue, InjectIntoNode node,
307+
static Object cachedLen(DynamicObjectStorage self, ForEachNode<Object> node, Object firstValue,
307308
@Exclusive @SuppressWarnings("unused") @Cached("self.store.getShape()") Shape cachedShape,
308309
@Cached(value = "keyList(self)", dimensions = 1) Object[] keys,
309310
@Shared("readNodeInject") @Cached ReadAttributeFromDynamicObjectNode readNode) {
310-
HashingStorage[] result = firstValue;
311+
Object result = firstValue;
311312
for (int i = 0; i < keys.length; i++) {
312313
Object key = keys[i];
313-
result = runNode(self, result, key, readNode, node);
314+
result = runNode(self, key, result, readNode, node);
314315
}
315316
return result;
316317
}
317318

318319
@Specialization(replaces = "cachedLen")
319-
static HashingStorage[] addAll(DynamicObjectStorage self, HashingStorage[] firstValue, InjectIntoNode node,
320+
static Object addAll(DynamicObjectStorage self, ForEachNode<Object> node, Object firstValue,
320321
@Shared("readNodeInject") @Cached ReadAttributeFromDynamicObjectNode readNode) {
321-
return cachedLen(self, firstValue, node, self.store.getShape(), keyList(self), readNode);
322+
return cachedLen(self, node, firstValue, self.store.getShape(), keyList(self), readNode);
322323
}
323324

324-
private static HashingStorage[] runNode(DynamicObjectStorage self, HashingStorage[] acc, Object key, ReadAttributeFromDynamicObjectNode readNode, InjectIntoNode node) {
325+
private static Object runNode(DynamicObjectStorage self, Object key, Object acc, ReadAttributeFromDynamicObjectNode readNode, ForEachNode<Object> node) {
325326
if (key instanceof String) {
326327
Object value = readNode.execute(self.store, key);
327328
if (value != PNone.NO_VALUE) {
328-
return node.execute(acc, key);
329+
return node.execute(key, acc);
329330
}
330331
}
331332
return acc;
@@ -348,9 +349,9 @@ public HashingStorage copy() {
348349
}
349350

350351
@ExportMessage
351-
public Iterator<Object> keys(
352+
public HashingStorageIterable<Object> keys(
352353
@Exclusive @Cached ReadAttributeFromDynamicObjectNode readNode) {
353-
return new KeysIterator(store, readNode);
354+
return new HashingStorageIterable<>(new KeysIterator(store, readNode));
354355
}
355356

356357
private static final class KeysIterator implements Iterator<Object> {
@@ -390,8 +391,9 @@ public Object next() {
390391
}
391392

392393
@ExportMessage
393-
public Iterator<DictEntry> entries(@Exclusive @Cached ReadAttributeFromDynamicObjectNode readNode) {
394-
return new EntriesIterator(store, readNode);
394+
public HashingStorageIterable<DictEntry> entries(
395+
@Exclusive @Cached ReadAttributeFromDynamicObjectNode readNode) {
396+
return new HashingStorageIterable<>(new EntriesIterator(store, readNode));
395397
}
396398

397399
private static final class EntriesIterator implements Iterator<DictEntry> {
@@ -401,11 +403,16 @@ private static final class EntriesIterator implements Iterator<DictEntry> {
401403
private DictEntry next = null;
402404

403405
public EntriesIterator(DynamicObject store, ReadAttributeFromDynamicObjectNode readNode) {
404-
this.keyIter = store.getShape().getKeys().iterator();
406+
this.keyIter = getIterator(store.getShape());
405407
this.store = store;
406408
this.readNode = readNode;
407409
}
408410

411+
@TruffleBoundary
412+
private static Iterator<Object> getIterator(Shape shape) {
413+
return shape.getKeys().iterator();
414+
}
415+
409416
@TruffleBoundary
410417
public boolean hasNext() {
411418
while (next == null && keyIter.hasNext()) {

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,9 @@
4646

4747
import java.util.Iterator;
4848

49-
import org.graalvm.collections.EconomicMap;
50-
import org.graalvm.collections.MapCursor;
51-
5249
import com.oracle.graal.python.builtins.objects.PNone;
53-
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.InjectIntoNode;
50+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.ForEachNode;
51+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
5452
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
5553
import com.oracle.graal.python.builtins.objects.object.PythonObject;
5654
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
@@ -74,6 +72,9 @@
7472
import com.oracle.truffle.api.profiles.ConditionProfile;
7573
import com.oracle.truffle.api.profiles.ValueProfile;
7674

75+
import org.graalvm.collections.EconomicMap;
76+
import org.graalvm.collections.MapCursor;
77+
7778
@ExportLibrary(HashingStorageLibrary.class)
7879
public class EconomicMapStorage extends HashingStorage {
7980

@@ -289,11 +290,11 @@ static HashingStorage setItemGeneric(EconomicMapStorage self, Object key, Object
289290

290291
@Override
291292
@ExportMessage
292-
public HashingStorage[] injectInto(HashingStorage[] firstValue, InjectIntoNode node) {
293-
HashingStorage[] result = firstValue;
293+
Object forEachUntyped(ForEachNode<Object> node, Object arg) {
294+
Object result = arg;
294295
MapCursor<DictKey, Object> cursor = map.getEntries();
295296
while (cursor.advance()) {
296-
result = node.execute(result, cursor.getKey().value);
297+
result = node.execute(cursor.getKey().value, result);
297298
}
298299
return result;
299300
}
@@ -575,8 +576,8 @@ static HashingStorage diffGeneric(EconomicMapStorage self, HashingStorage other,
575576

576577
@Override
577578
@ExportMessage
578-
public Iterator<Object> keys() {
579-
return new KeysIterator(map.getKeys().iterator());
579+
public HashingStorageIterable<Object> keys() {
580+
return new HashingStorageIterable<>(new KeysIterator(map.getKeys().iterator()));
580581
}
581582

582583
private static final class KeysIterator implements Iterator<Object> {

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
import java.util.Iterator;
4444
import java.util.NoSuchElementException;
4545

46-
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.InjectIntoNode;
46+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.ForEachNode;
47+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
4748
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
4849
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
4950
import com.oracle.truffle.api.dsl.Cached;
@@ -100,8 +101,8 @@ public HashingStorage delItemWithState(Object key, ThreadState state,
100101

101102
@Override
102103
@ExportMessage
103-
public HashingStorage[] injectInto(HashingStorage[] firstValue, @SuppressWarnings("unused") InjectIntoNode node) {
104-
return firstValue;
104+
Object forEachUntyped(@SuppressWarnings("unused") ForEachNode<Object> node, Object arg) {
105+
return arg;
105106
}
106107

107108
@Override
@@ -128,7 +129,7 @@ public Object next() {
128129

129130
@Override
130131
@ExportMessage
131-
public Iterator<Object> keys() {
132-
return KEYS_ITERATOR;
132+
public HashingStorageIterable<Object> keys() {
133+
return new HashingStorageIterable<>(KEYS_ITERATOR);
133134
}
134135
}

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@
5151
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
5252
import com.oracle.graal.python.builtins.objects.PNone;
5353
import com.oracle.graal.python.builtins.objects.common.HashingStorageFactory.InitNodeGen;
54+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.ForEachNode;
55+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
5456
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.InjectIntoNode;
5557
import com.oracle.graal.python.builtins.objects.dict.PDict;
5658
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
@@ -320,7 +322,7 @@ HashingStorage delItemWithState(Object key, ThreadState state) {
320322

321323
@SuppressWarnings({"unused", "static-method"})
322324
@ExportMessage
323-
HashingStorage[] injectInto(HashingStorage[] firstValue, InjectIntoNode node) {
325+
Object forEachUntyped(ForEachNode<Object> node, Object arg) {
324326
throw new AbstractMethodError("HashingStorage.injectInto");
325327
}
326328

@@ -338,7 +340,7 @@ HashingStorage copy() {
338340

339341
@SuppressWarnings({"unused", "static-method"})
340342
@ExportMessage
341-
Iterator<Object> keys() {
343+
HashingStorageIterable<Object> keys() {
342344
throw new AbstractMethodError("HashingStorage.keys");
343345
}
344346

@@ -544,8 +546,8 @@ public HashingStorage diff(HashingStorage other,
544546
}
545547

546548
@ExportMessage
547-
public Iterator<Object> values(@CachedLibrary("this") HashingStorageLibrary lib) {
548-
return new ValuesIterator(this, lib);
549+
public HashingStorageIterable<Object> values(@CachedLibrary("this") HashingStorageLibrary lib) {
550+
return new HashingStorageIterable<>(new ValuesIterator(this, lib));
549551
}
550552

551553
private static final class ValuesIterator implements Iterator<Object> {
@@ -565,19 +567,19 @@ public Object next() {
565567
}
566568

567569
@ExportMessage
568-
public Iterator<DictEntry> entries(@CachedLibrary("this") HashingStorageLibrary lib) {
569-
return new EntriesIterator(this, lib);
570+
public HashingStorageIterable<DictEntry> entries(@CachedLibrary("this") HashingStorageLibrary lib) {
571+
return new HashingStorageIterable<>(new EntriesIterator(this, lib));
570572
}
571573

572574
private static final class EntriesIterator implements Iterator<DictEntry> {
573-
private final Iterator<Object> keysIterator;
575+
private final HashingStorageIterable.BoundaryIterator<Object> keysIterator;
574576
private final HashingStorage self;
575577
private final HashingStorageLibrary lib;
576578

577579
EntriesIterator(HashingStorage self, HashingStorageLibrary lib) {
578580
this.self = self;
579581
this.lib = lib;
580-
this.keysIterator = lib.keys(self);
582+
this.keysIterator = lib.keys(self).iterator();
581583
}
582584

583585
public boolean hasNext() {

0 commit comments

Comments
 (0)