Skip to content

Commit 2bd2aaa

Browse files
steve-ssteve-s
committed
[GR-23657] Add helper methods to libraries to avoid if (has-frame) fooWithState(...) else foo(...) pattern.
PullRequest: graalpython/967
2 parents 2734839 + 48ad1d9 commit 2bd2aaa

19 files changed

+225
-371
lines changed

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

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@
7676
import java.math.BigInteger;
7777
import java.nio.CharBuffer;
7878
import java.util.List;
79-
import com.oracle.graal.python.util.Supplier;
8079

8180
import com.oracle.graal.python.PythonLanguage;
8281
import com.oracle.graal.python.builtins.Builtin;
@@ -162,6 +161,7 @@
162161
import com.oracle.graal.python.runtime.exception.PException;
163162
import com.oracle.graal.python.runtime.exception.PythonErrorType;
164163
import com.oracle.graal.python.runtime.sequence.PSequence;
164+
import com.oracle.graal.python.util.Supplier;
165165
import com.oracle.truffle.api.Assumption;
166166
import com.oracle.truffle.api.CallTarget;
167167
import com.oracle.truffle.api.CompilerDirectives;
@@ -304,12 +304,7 @@ String doO(VirtualFrame frame, Object x,
304304
@Cached BranchProfile isInt,
305305
@Cached BranchProfile isLong,
306306
@Cached BranchProfile isPInt) {
307-
Object index;
308-
if (hasFrame.profile(frame != null)) {
309-
index = lib.asIndexWithState(x, PArguments.getThreadState(frame));
310-
} else {
311-
index = lib.asIndex(x);
312-
}
307+
Object index = lib.asIndexWithFrame(x, hasFrame, frame);
313308
if (isSubtype.execute(lib.getLazyPythonClass(index), PythonBuiltinClassType.PInt)) {
314309
if (index instanceof Boolean || index instanceof Integer) {
315310
isInt.enter();
@@ -460,11 +455,7 @@ public abstract static class HashNode extends PythonUnaryBuiltinNode {
460455
long hash(VirtualFrame frame, Object object,
461456
@Cached("createBinaryProfile()") ConditionProfile profile,
462457
@CachedLibrary("object") PythonObjectLibrary lib) {
463-
if (profile.profile(frame != null)) {
464-
return lib.hashWithState(object, PArguments.getThreadState(frame));
465-
} else {
466-
return lib.hash(object);
467-
}
458+
return lib.hashWithFrame(object, profile, frame);
468459
}
469460
}
470461

@@ -1220,11 +1211,7 @@ public abstract static class LenNode extends PythonUnaryBuiltinNode {
12201211
public int len(VirtualFrame frame, Object obj,
12211212
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
12221213
@CachedLibrary("obj") PythonObjectLibrary lib) {
1223-
if (hasFrame.profile(frame != null)) {
1224-
return lib.lengthWithState(obj, PArguments.getThreadState(frame));
1225-
} else {
1226-
return lib.length(obj);
1227-
}
1214+
return lib.lengthWithFrame(obj, hasFrame, frame);
12281215
}
12291216
}
12301217

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@
6666
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
6767
import com.oracle.graal.python.builtins.objects.dict.PDict;
6868
import com.oracle.graal.python.builtins.objects.exception.PBaseException;
69-
import com.oracle.graal.python.builtins.objects.function.PArguments;
7069
import com.oracle.graal.python.builtins.objects.ints.PInt;
7170
import com.oracle.graal.python.builtins.objects.module.PythonModule;
7271
import com.oracle.graal.python.builtins.objects.object.PythonObject;
@@ -379,9 +378,7 @@ public int run(VirtualFrame frame, String name,
379378
@Cached("createBinaryProfile()") ConditionProfile hasFrame) {
380379
if (getCore().lookupBuiltinModule(name) != null) {
381380
return 1;
382-
} else if (isWithinContext() && hasFrame.profile(frame != null) && hasKeyLib.hasKeyWithState(getStorage(), name, PArguments.getThreadState(frame))) {
383-
return -1;
384-
} else if (isWithinContext() && hasKeyLib.hasKey(getStorage(), name)) {
381+
} else if (isWithinContext() && hasKeyLib.hasKeyWithFrame(getStorage(), name, hasFrame, frame)) {
385382
return -1;
386383
} else {
387384
return 0;

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -414,12 +414,7 @@ void handlePDict(VirtualFrame frame, PDict d, int version, DataOutputStream buff
414414
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
415415
@CachedLibrary("d.getDictStorage()") HashingStorageLibrary lib) {
416416
writeByte(TYPE_DICT, version, buffer);
417-
int len;
418-
if (hasFrame.profile(frame != null)) {
419-
len = lib.lengthWithState(d.getDictStorage(), PArguments.getThreadState(frame));
420-
} else {
421-
len = lib.length(d.getDictStorage());
422-
}
417+
int len = lib.lengthWithFrame(d.getDictStorage(), hasFrame, frame);
423418
writeInt(len, version, buffer);
424419
for (DictEntry entry : d.entries()) {
425420
getRecursiveNode().execute(frame, entry.key, version, buffer);

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
import com.oracle.graal.python.builtins.objects.PNone;
4545
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
4646
import com.oracle.graal.python.builtins.objects.floats.PFloat;
47-
import com.oracle.graal.python.builtins.objects.function.PArguments;
48-
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
4947
import com.oracle.graal.python.builtins.objects.function.PKeyword;
5048
import com.oracle.graal.python.builtins.objects.ints.PInt;
5149
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
@@ -1240,15 +1238,8 @@ Object gcd(VirtualFrame frame, Object x, Object y,
12401238
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
12411239
@CachedLibrary(limit = "2") PythonObjectLibrary lib,
12421240
@Cached("create()") GcdNode recursiveNode) {
1243-
Object xValue, yValue;
1244-
if (hasFrame.profile(frame != null)) {
1245-
ThreadState threadState = PArguments.getThreadState(frame);
1246-
xValue = lib.asIndexWithState(x, threadState);
1247-
yValue = lib.asIndexWithState(y, threadState);
1248-
} else {
1249-
xValue = lib.asIndex(x);
1250-
yValue = lib.asIndex(y);
1251-
}
1241+
Object xValue = lib.asIndexWithFrame(x, hasFrame, frame);
1242+
Object yValue = lib.asIndexWithFrame(y, hasFrame, frame);
12521243
return recursiveNode.execute(frame, xValue, yValue);
12531244
}
12541245

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodesFactory.GetDictStorageNodeGen;
4444
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodesFactory.LenNodeGen;
4545
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodesFactory.SetItemNodeGen;
46-
import com.oracle.graal.python.builtins.objects.function.PArguments;
4746
import com.oracle.graal.python.nodes.PGuards;
4847
import com.oracle.graal.python.nodes.PNodeWithContext;
4948
import com.oracle.truffle.api.dsl.Cached;
@@ -89,11 +88,7 @@ void doSetItemCached(VirtualFrame frame, PHashingCollection c, Object key, Objec
8988
@CachedLibrary("c.getDictStorage()") HashingStorageLibrary lib,
9089
@Cached("c.getClass()") Class<? extends PHashingCollection> cachedClass) {
9190
HashingStorage storage = cachedClass.cast(c).getDictStorage();
92-
if (hasFrame.profile(frame != null)) {
93-
storage = lib.setItemWithState(storage, key, value, PArguments.getThreadState(frame));
94-
} else {
95-
storage = lib.setItem(storage, key, value);
96-
}
91+
storage = lib.setItemWithFrame(storage, key, value, hasFrame, frame);
9792
cachedClass.cast(c).setDictStorage(storage);
9893
}
9994

@@ -102,11 +97,7 @@ void doSetItemGeneric(VirtualFrame frame, PHashingCollection c, Object key, Obje
10297
@Cached("createBinaryProfile()") ConditionProfile hasFrame,
10398
@CachedLibrary("c.getDictStorage()") HashingStorageLibrary lib) {
10499
HashingStorage storage = c.getDictStorage();
105-
if (hasFrame.profile(frame != null)) {
106-
storage = lib.setItemWithState(storage, key, value, PArguments.getThreadState(frame));
107-
} else {
108-
storage = lib.setItem(storage, key, value);
109-
}
100+
storage = lib.setItemWithFrame(storage, key, value, hasFrame, frame);
110101
c.setDictStorage(storage);
111102
}
112103

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,16 @@
4242

4343
import java.util.Iterator;
4444

45+
import com.oracle.graal.python.builtins.objects.function.PArguments;
4546
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
4647
import com.oracle.truffle.api.CompilerDirectives;
4748
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
49+
import com.oracle.truffle.api.frame.VirtualFrame;
4850
import com.oracle.truffle.api.library.GenerateLibrary;
4951
import com.oracle.truffle.api.library.Library;
5052
import com.oracle.truffle.api.library.LibraryFactory;
5153
import com.oracle.truffle.api.nodes.Node;
54+
import com.oracle.truffle.api.profiles.ConditionProfile;
5255

5356
/*
5457
* TODO:
@@ -92,6 +95,17 @@ public int length(HashingStorage self) {
9295
return lengthWithState(self, null);
9396
}
9497

98+
/**
99+
* @see #lengthWithState(HashingStorage, ThreadState)
100+
*/
101+
public final int lengthWithFrame(HashingStorage self, ConditionProfile hasFrameProfile, VirtualFrame frame) {
102+
if (hasFrameProfile.profile(frame != null)) {
103+
return lengthWithState(self, PArguments.getThreadState(frame));
104+
} else {
105+
return length(self);
106+
}
107+
}
108+
95109
/**
96110
* Implementers <i>must</i> call {@code __hash__} on the key if that could be visible, to comply
97111
* with Python semantics.
@@ -110,6 +124,17 @@ public boolean hasKey(HashingStorage self, Object key) {
110124
return getItem(self, key) != null;
111125
}
112126

127+
/**
128+
* @see #hasKeyWithState(HashingStorage, Object, ThreadState)
129+
*/
130+
public final boolean hasKeyWithFrame(HashingStorage self, Object key, ConditionProfile hasFrameProfile, VirtualFrame frame) {
131+
if (hasFrameProfile.profile(frame != null)) {
132+
return hasKeyWithState(self, key, PArguments.getThreadState(frame));
133+
} else {
134+
return hasKey(self, key);
135+
}
136+
}
137+
113138
/**
114139
* Implementers <i>must</i> call {@code __hash__} on the key if that could be visible, to comply
115140
* with Python semantics.
@@ -132,6 +157,17 @@ public Object getItem(HashingStorage self, Object key) {
132157
return getItemWithState(self, key, null);
133158
}
134159

160+
/**
161+
* @see #getItemWithState(HashingStorage, Object, ThreadState)
162+
*/
163+
public final Object getItemWithFrame(HashingStorage self, Object key, ConditionProfile hasFrameProfile, VirtualFrame frame) {
164+
if (hasFrameProfile.profile(frame != null)) {
165+
return getItemWithState(self, key, PArguments.getThreadState(frame));
166+
} else {
167+
return getItem(self, key);
168+
}
169+
}
170+
135171
/**
136172
* Implementers <i>must</i> call {@code __hash__} on the key if that could be visible, to comply
137173
* with Python semantics.
@@ -155,6 +191,17 @@ public final HashingStorage setItem(HashingStorage self, Object key, Object valu
155191
return setItemWithState(self, key, value, null);
156192
}
157193

194+
/**
195+
* @see #setItemWithState(HashingStorage, Object, Object, ThreadState)
196+
*/
197+
public final HashingStorage setItemWithFrame(HashingStorage self, Object key, Object value, ConditionProfile hasFrameProfile, VirtualFrame frame) {
198+
if (hasFrameProfile.profile(frame != null)) {
199+
return setItemWithState(self, key, value, PArguments.getThreadState(frame));
200+
} else {
201+
return setItem(self, key, value);
202+
}
203+
}
204+
158205
/**
159206
* Implementers <i>must</i> call {@code __hash__} on the key if that could be visible, to comply
160207
* with Python semantics.
@@ -174,6 +221,14 @@ public HashingStorage delItem(HashingStorage self, Object key) {
174221
return delItemWithState(self, key, null);
175222
}
176223

224+
public final HashingStorage delItemWithFrame(HashingStorage self, Object key, ConditionProfile hasFrameProfile, VirtualFrame frame) {
225+
if (hasFrameProfile.profile(frame != null)) {
226+
return delItemWithState(self, key, PArguments.getThreadState(frame));
227+
} else {
228+
return delItem(self, key);
229+
}
230+
}
231+
177232
/**
178233
* A node to be called in a loop with different keys to operate on {@code
179234
* store}. It's execute method returns the new store to use for the next iteration.
@@ -275,6 +330,17 @@ public int compareKeys(HashingStorage self, HashingStorage other) {
275330
return compareKeysWithState(self, other, null);
276331
}
277332

333+
/**
334+
* @see #compareKeysWithState(HashingStorage, HashingStorage, ThreadState)
335+
*/
336+
public final int compareKeysWithFrame(HashingStorage self, HashingStorage other, ConditionProfile hasFrameProfile, VirtualFrame frame) {
337+
if (hasFrameProfile.profile(frame != null)) {
338+
return compareKeysWithState(self, other, PArguments.getThreadState(frame));
339+
} else {
340+
return compareKeys(self, other);
341+
}
342+
}
343+
278344
/**
279345
* @return {@code 0} if all entries are equal, {@code -1} if {@code self} is a subset, or
280346
* {@code 1} if neither.
@@ -312,6 +378,17 @@ public HashingStorage intersect(HashingStorage self, HashingStorage other) {
312378
return intersectWithState(self, other, null);
313379
}
314380

381+
/**
382+
* @see #intersectWithState(HashingStorage, HashingStorage, ThreadState)
383+
*/
384+
public final HashingStorage intersectWithFrame(HashingStorage self, HashingStorage other, ConditionProfile hasFrameProfile, VirtualFrame frame) {
385+
if (hasFrameProfile.profile(frame != null)) {
386+
return intersectWithState(self, other, PArguments.getThreadState(frame));
387+
} else {
388+
return intersect(self, other);
389+
}
390+
}
391+
315392
/**
316393
* @return {@code true} iff the intersection of the two sets is empty.
317394
*/
@@ -359,6 +436,17 @@ public HashingStorage diff(HashingStorage self, HashingStorage other) {
359436
return diffWithState(self, other, null);
360437
}
361438

439+
/**
440+
* @see #getItemWithState(HashingStorage, Object, ThreadState)
441+
*/
442+
public final HashingStorage diffWithFrame(HashingStorage self, HashingStorage other, ConditionProfile hasFrameProfile, VirtualFrame frame) {
443+
if (hasFrameProfile.profile(frame != null)) {
444+
return diffWithState(self, other, PArguments.getThreadState(frame));
445+
} else {
446+
return diff(self, other);
447+
}
448+
}
449+
362450
/**
363451
* An iterable that does not need to be guarded separately with TruffleBoundary.
364452
*/

0 commit comments

Comments
 (0)