Skip to content

Commit 1de9e02

Browse files
tomasstupkatomasstupka
committed
avoid not necessary uncached calls of HashingStorageLibrary.keys/entries()
1 parent 22b8444 commit 1de9e02

File tree

4 files changed

+38
-25
lines changed

4 files changed

+38
-25
lines changed

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.oracle.graal.python.builtins.objects.code.CodeNodes.CreateCodeNode;
5252
import com.oracle.graal.python.builtins.objects.code.PCode;
5353
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
54+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.GetDictStorageNode;
5455
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
5556
import com.oracle.graal.python.builtins.objects.common.HashingStorage.DictEntry;
5657
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
@@ -412,11 +413,13 @@ void handlePList(VirtualFrame frame, PList l, int version, DataOutputStream buff
412413
@Specialization(limit = "1")
413414
void handlePDict(VirtualFrame frame, PDict d, int version, DataOutputStream buffer,
414415
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
415-
@CachedLibrary("d.getDictStorage()") HashingStorageLibrary lib) {
416+
@Cached GetDictStorageNode getStore,
417+
@CachedLibrary("getStore.execute(d)") HashingStorageLibrary lib) {
416418
writeByte(TYPE_DICT, version, buffer);
417-
int len = lib.lengthWithFrame(d.getDictStorage(), hasFrame, frame);
419+
HashingStorage dictStorage = getStore.execute(d);
420+
int len = lib.lengthWithFrame(dictStorage, hasFrame, frame);
418421
writeInt(len, version, buffer);
419-
for (DictEntry entry : d.entries()) {
422+
for (DictEntry entry : lib.entries(dictStorage)) {
420423
getRecursiveNode().execute(frame, entry.key, version, buffer);
421424
getRecursiveNode().execute(frame, entry.value, version, buffer);
422425
}
@@ -443,33 +446,37 @@ private static String getSourceCode(PCode c) {
443446
@Specialization(limit = "1")
444447
void handlePSet(VirtualFrame frame, PSet s, int version, DataOutputStream buffer,
445448
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
446-
@CachedLibrary("s.getDictStorage()") HashingStorageLibrary lib) {
449+
@Cached GetDictStorageNode getStore,
450+
@CachedLibrary("getStore.execute(s)") HashingStorageLibrary lib) {
447451
writeByte(TYPE_SET, version, buffer);
448452
int len;
453+
HashingStorage dictStorage = getStore.execute(s);
449454
if (hasFrame.profile(frame != null)) {
450-
len = lib.lengthWithState(s.getDictStorage(), PArguments.getThreadState(frame));
455+
len = lib.lengthWithState(dictStorage, PArguments.getThreadState(frame));
451456
} else {
452-
len = lib.length(s.getDictStorage());
457+
len = lib.length(dictStorage);
453458
}
454459
writeInt(len, version, buffer);
455-
for (DictEntry entry : s.entries()) {
460+
for (DictEntry entry : lib.entries(dictStorage)) {
456461
getRecursiveNode().execute(frame, entry.key, version, buffer);
457462
}
458463
}
459464

460465
@Specialization(limit = "1")
461466
void handlePForzenSet(VirtualFrame frame, PFrozenSet s, int version, DataOutputStream buffer,
462467
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
463-
@CachedLibrary("s.getDictStorage()") HashingStorageLibrary lib) {
468+
@Cached GetDictStorageNode getStore,
469+
@CachedLibrary("getStore.execute(s)") HashingStorageLibrary lib) {
464470
writeByte(TYPE_FROZENSET, version, buffer);
465471
int len;
472+
HashingStorage dictStorage = getStore.execute(s);
466473
if (hasFrame.profile(frame != null)) {
467-
len = lib.lengthWithState(s.getDictStorage(), PArguments.getThreadState(frame));
474+
len = lib.lengthWithState(dictStorage, PArguments.getThreadState(frame));
468475
} else {
469-
len = lib.length(s.getDictStorage());
476+
len = lib.length(dictStorage);
470477
}
471478
writeInt(len, version, buffer);
472-
for (DictEntry entry : s.entries()) {
479+
for (DictEntry entry : lib.entries(dictStorage)) {
473480
getRecursiveNode().execute(frame, entry.key, version, buffer);
474481
}
475482
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
4040
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
4141
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
42-
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
4342
import com.oracle.graal.python.builtins.objects.dict.PDictView.PDictValuesView;
4443
import static com.oracle.graal.python.nodes.SpecialMethodNames.__REVERSED__;
4544
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
@@ -99,12 +98,13 @@ public abstract static class EqNode extends PythonBuiltinNode {
9998
@Specialization(limit = "1")
10099
boolean doItemsView(VirtualFrame frame, PDictValuesView self, PDictValuesView other,
101100
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
102-
@CachedLibrary("other.getWrappedDict().getDictStorage()") HashingStorageLibrary lib) {
101+
@Cached HashingCollectionNodes.GetDictStorageNode getStore,
102+
@CachedLibrary("getStore.execute(self.getWrappedDict())") HashingStorageLibrary libSelf,
103+
@CachedLibrary("getStore.execute(other.getWrappedDict())") HashingStorageLibrary libOther) {
103104

104-
final PHashingCollection dict = self.getWrappedDict();
105-
final HashingStorage storage = other.getWrappedDict().getDictStorage();
106-
for (Object selfKey : dict.keys()) {
107-
final boolean hasKey = lib.hasKeyWithFrame(storage, selfKey, hasFrame, frame);
105+
final HashingStorage storage = getStore.execute(other.getWrappedDict());
106+
for (Object selfKey : libSelf.keys(getStore.execute(self.getWrappedDict()))) {
107+
final boolean hasKey = libOther.hasKeyWithFrame(storage, selfKey, hasFrame, frame);
108108
if (!hasKey) {
109109
return false;
110110
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/mappingproxy/MappingproxyBuiltins.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.oracle.graal.python.builtins.PythonBuiltins;
4646
import com.oracle.graal.python.builtins.objects.PNone;
4747
import com.oracle.graal.python.builtins.objects.PNotImplemented;
48+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.GetDictStorageNode;
4849
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
4950
import com.oracle.graal.python.builtins.objects.dict.PDict;
5051
import com.oracle.graal.python.builtins.objects.dict.PDictView;
@@ -86,9 +87,11 @@ Object doPMappingproxy(PMappingproxy self, Object mapping) {
8687
@Builtin(name = __ITER__, minNumOfPositionalArgs = 1)
8788
@GenerateNodeFactory
8889
public abstract static class IterNode extends PythonUnaryBuiltinNode {
89-
@Specialization
90-
Object run(PMappingproxy self) {
91-
return factory().createDictKeyIterator(self.keys().iterator());
90+
@Specialization(limit = "1")
91+
Object run(PMappingproxy self,
92+
@Cached GetDictStorageNode getStore,
93+
@CachedLibrary("getStore.execute(self)") HashingStorageLibrary lib) {
94+
return factory().createDictKeyIterator(lib.keys(getStore.execute(self)).iterator());
9295
}
9396
}
9497

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/generator/DictConcatNode.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.graal.python.nodes.generator;
4242

43+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.GetDictStorageNode;
4344
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
4445
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
4546
import com.oracle.graal.python.builtins.objects.dict.PDict;
@@ -66,6 +67,7 @@ protected DictConcatNode(ExpressionNode... mappablesNodes) {
6667
@Specialization
6768
public Object concat(VirtualFrame frame,
6869
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
70+
@Cached GetDictStorageNode getStore,
6971
@CachedLibrary(limit = "2") HashingStorageLibrary firstlib,
7072
@CachedLibrary(limit = "1") HashingStorageLibrary otherlib) {
7173
PDict first = null;
@@ -75,17 +77,18 @@ public Object concat(VirtualFrame frame,
7577
first = expectDict(n.execute(frame));
7678
} else {
7779
other = expectDict(n.execute(frame));
78-
addAllToDict(frame, first, other, hasFrame, firstlib, otherlib);
80+
addAllToDict(frame, first, other, hasFrame, firstlib, otherlib, getStore);
7981
}
8082
}
8183
return first;
8284
}
8385

8486
private static void addAllToDict(VirtualFrame frame, PDict dict, PDict other, ConditionProfile hasFrame,
85-
HashingStorageLibrary firstlib, HashingStorageLibrary otherlib) {
86-
HashingStorage dictStorage = dict.getDictStorage();
87-
for (Object key : other.keys()) {
88-
Object value = otherlib.getItemWithFrame(other.getDictStorage(), key, hasFrame, frame);
87+
HashingStorageLibrary firstlib, HashingStorageLibrary otherlib, GetDictStorageNode getStore) {
88+
HashingStorage dictStorage = getStore.execute(dict);
89+
HashingStorage otherStorage = getStore.execute(other);
90+
for (Object key : otherlib.keys(otherStorage)) {
91+
Object value = otherlib.getItemWithFrame(otherStorage, key, hasFrame, frame);
8992
dictStorage = firstlib.setItemWithFrame(dictStorage, key, value, hasFrame, frame);
9093
}
9194
dict.setDictStorage(dictStorage);

0 commit comments

Comments
 (0)