Skip to content

Commit fe790de

Browse files
committed
[GR-21417] Make iterating over hashes more convenient
PullRequest: graalpython/869
2 parents 636f280 + 55bdda6 commit fe790de

25 files changed

+310
-215
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: 48 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,55 @@ 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+
@GenerateUncached
960+
static final class EachSubclassAdd extends HashingStorageLibrary.ForEachNode<SubclassAddState> {
961+
962+
private static final EachSubclassAdd UNCACHED = new EachSubclassAdd();
963+
964+
@Override
965+
public SubclassAddState execute(Object key, SubclassAddState s) {
966+
setAdd(s.subclasses, (PythonClass) s.lib.getItem(s.storage, key));
967+
return s;
968+
}
969+
970+
@TruffleBoundary
971+
protected static void setAdd(Set<PythonAbstractClass> set, PythonClass cls) {
972+
set.add(cls);
973+
}
974+
975+
static EachSubclassAdd create() {
976+
return new EachSubclassAdd();
977+
}
978+
979+
static EachSubclassAdd getUncached() {
980+
return UNCACHED;
981+
}
982+
}
983+
944984
@Specialization(guards = "eq(TP_SUBCLASSES, key)", limit = "1")
945-
@TruffleBoundary
946985
Object doTpSubclasses(PythonClass object, @SuppressWarnings("unused") String key, DynamicObjectNativeWrapper.PythonObjectNativeWrapper value,
947-
@CachedLibrary("value") PythonNativeWrapperLibrary lib) {
948-
// TODO more type checking; do fast path
986+
@Cached GetSubclassesNode getSubclassesNode,
987+
@Cached EachSubclassAdd eachNode,
988+
@Cached HashingCollectionNodes.GetDictStorageNode getStorage,
989+
@CachedLibrary("value") PythonNativeWrapperLibrary lib,
990+
@CachedLibrary(limit = "1") HashingStorageLibrary hashLib) {
949991
PDict dict = (PDict) lib.getDelegate(value);
950-
for (Object v : dict.items()) {
951-
GetSubclassesNode.doSlowPath(object).add((PythonClass) v);
952-
}
992+
HashingStorage storage = getStorage.execute(dict);
993+
Set<PythonAbstractClass> subclasses = getSubclassesNode.execute(object);
994+
hashLib.forEach(storage, eachNode, new SubclassAddState(storage, hashLib, subclasses));
953995
return value;
954996
}
955997

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)