Skip to content

Commit 50853e3

Browse files
qunaibitqunaibit
committed
HashingStorageNode refactors
1 parent e631566 commit 50853e3

File tree

11 files changed

+221
-189
lines changed

11 files changed

+221
-189
lines changed

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
import com.oracle.graal.python.builtins.objects.code.CodeNodes;
9696
import com.oracle.graal.python.builtins.objects.code.PCode;
9797
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
98+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
9899
import com.oracle.graal.python.builtins.objects.common.HashingStorage.DictEntry;
99100
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
100101
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
@@ -2267,7 +2268,6 @@ public abstract static class TypeNode extends PythonBuiltinNode {
22672268
@Child private SequenceStorageNodes.GetItemNode getItemNode;
22682269
@Child private SequenceStorageNodes.AppendNode appendNode;
22692270
@Child private HashingStorageNodes.ContainsKeyNode containsKeyNode;
2270-
@Child private HashingStorageNodes.GetItemNode getDictItemNode;
22712271
@Child private CExtNodes.PCallCapiFunction callAddNativeSlotsNode;
22722272
@Child private CExtNodes.ToSulongNode toSulongNode;
22732273
@Child private ReadCallerFrameNode readCallerFrameNode;
@@ -2287,6 +2287,7 @@ Object type(Object cls, Object obj, PNone bases, PNone dict, PKeyword[] kwds,
22872287
@Specialization
22882288
Object type(VirtualFrame frame, LazyPythonClass cls, String name, PTuple bases, PDict namespace, PKeyword[] kwds,
22892289
@Cached GetLazyClassNode getMetaclassNode,
2290+
@CachedLibrary(limit = "1") HashingStorageLibrary hlib,
22902291
@Cached("create(__NEW__)") LookupInheritedAttributeNode getNewFuncNode,
22912292
@Cached("create(__INIT_SUBCLASS__)") GetAttributeNode getInitSubclassNode,
22922293
@Cached HashingStorageNodes.GetItemNode getClasscellNode,
@@ -2322,7 +2323,7 @@ Object type(VirtualFrame frame, LazyPythonClass cls, String name, PTuple bases,
23222323
PFrame callerFrame = getReadCallerFrameNode().executeWith(frame, 0);
23232324
PythonObject globals = callerFrame.getGlobals();
23242325
if (globals != null) {
2325-
String moduleName = getModuleNameFromGlobals(frame, globals);
2326+
String moduleName = getModuleNameFromGlobals(frame, globals, hlib);
23262327
if (moduleName != null) {
23272328
ensureWriteAttrNode().execute(frame, newType, __MODULE__, moduleName);
23282329
}
@@ -2346,12 +2347,12 @@ Object type(VirtualFrame frame, LazyPythonClass cls, String name, PTuple bases,
23462347
}
23472348
}
23482349

2349-
private String getModuleNameFromGlobals(VirtualFrame frame, PythonObject globals) {
2350+
private String getModuleNameFromGlobals(VirtualFrame frame, PythonObject globals, HashingStorageLibrary hlib) {
23502351
Object nameAttr;
23512352
if (globals instanceof PythonModule) {
23522353
nameAttr = ensureReadAttrNode().execute(globals, __NAME__);
23532354
} else if (globals instanceof PDict) {
2354-
nameAttr = ensureGetDictItemNode().execute(frame, ((PDict) globals).getDictStorage(), __NAME__);
2355+
nameAttr = hlib.getItem(((PDict) globals).getDictStorage(), __NAME__);
23552356
} else {
23562357
CompilerDirectives.transferToInterpreter();
23572358
throw new IllegalStateException("invalid globals object");
@@ -2690,14 +2691,6 @@ Object typeGeneric(VirtualFrame frame, Object cls, Object name, Object bases, Ob
26902691
return nextTypeNode.execute(frame, cls, name, bases, dict, kwds);
26912692
}
26922693

2693-
private HashingStorageNodes.GetItemNode ensureGetDictItemNode() {
2694-
if (getDictItemNode == null) {
2695-
CompilerDirectives.transferToInterpreterAndInvalidate();
2696-
getDictItemNode = insert(HashingStorageNodes.GetItemNode.create());
2697-
}
2698-
return getDictItemNode;
2699-
}
2700-
27012694
private ReadAttributeFromObjectNode ensureReadAttrNode() {
27022695
if (readAttrNode == null) {
27032696
CompilerDirectives.transferToInterpreterAndInvalidate();

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@
9494
import com.oracle.graal.python.builtins.objects.bytes.PIBytesLike;
9595
import com.oracle.graal.python.builtins.objects.code.PCode;
9696
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
97-
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
97+
import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary;
9898
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
9999
import com.oracle.graal.python.builtins.objects.common.SequenceNodes;
100100
import com.oracle.graal.python.builtins.objects.common.SequenceNodes.GetObjectArrayNode;
@@ -1617,24 +1617,23 @@ Object setAttr(VirtualFrame frame, Object object, Object key, Object value,
16171617
@Builtin(name = BREAKPOINT, takesVarArgs = true, takesVarKeywordArgs = true)
16181618
@GenerateNodeFactory
16191619
public abstract static class BreakPointNode extends PythonBuiltinNode {
1620-
@Child private HashingStorageNodes.GetItemNode getSysModuleNode;
16211620
@Child private ReadAttributeFromObjectNode getBreakpointhookNode;
16221621
@Child private CallNode callNode;
16231622

16241623
@Specialization
1625-
public Object doIt(VirtualFrame frame, Object[] args, PKeyword[] kwargs) {
1624+
public Object doIt(VirtualFrame frame, Object[] args, PKeyword[] kwargs,
1625+
@CachedLibrary("getContext().getSysModules().getDictStorage()") HashingStorageLibrary hlib) {
16261626
if (getDebuggerSessionCount() > 0) {
16271627
// we already have a Truffle debugger attached, it'll stop here
16281628
return PNone.NONE;
16291629
} else if (getContext().isInitialized()) {
16301630
if (callNode == null) {
16311631
CompilerDirectives.transferToInterpreterAndInvalidate();
1632-
getSysModuleNode = insert(HashingStorageNodes.GetItemNode.create());
16331632
getBreakpointhookNode = insert(ReadAttributeFromObjectNode.create());
16341633
callNode = insert(CallNode.create());
16351634
}
16361635
PDict sysModules = getContext().getSysModules();
1637-
Object sysModule = getSysModuleNode.execute(frame, sysModules.getDictStorage(), "sys");
1636+
Object sysModule = hlib.getItem(sysModules.getDictStorage(), "sys");
16381637
Object breakpointhook = getBreakpointhookNode.execute(sysModule, BREAKPOINTHOOK);
16391638
if (breakpointhook == PNone.NO_VALUE) {
16401639
throw raise(PythonBuiltinClassType.RuntimeError, "lost sys.breakpointhook");

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,22 +146,22 @@ private static int incrementLen(DynamicObjectStorage self, ReadAttributeFromDyna
146146
public static class GetItemWithState {
147147
@Specialization
148148
static Object string(DynamicObjectStorage self, String key, ThreadState state,
149-
@Shared("readNodeGetItem") @Cached ReadAttributeFromDynamicObjectNode readNode) {
150-
Object result = readNode.execute(self.store, key);
149+
@Shared("readKey") @Cached ReadAttributeFromDynamicObjectNode readKey) {
150+
Object result = readKey.execute(self.store, key);
151151
return result == PNone.NO_VALUE ? null : result;
152152
}
153153

154154
@Specialization(guards = "isBuiltinString(key, profile)", limit = "1")
155155
static Object pstring(DynamicObjectStorage self, PString key, ThreadState state,
156-
@Shared("readNodeGetItem") @Cached ReadAttributeFromDynamicObjectNode readNode,
156+
@Shared("readKey") @Cached ReadAttributeFromDynamicObjectNode readKey,
157157
@Shared("builtinStringProfile") @Cached IsBuiltinClassProfile profile) {
158-
return string(self, key.getValue(), state, readNode);
158+
return string(self, key.getValue(), state, readKey);
159159
}
160160

161161
@Specialization(guards = {"cachedShape == self.store.getShape()", "!isBuiltinString(key, profile)"}, limit = "1")
162162
@ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN)
163163
static Object notString(DynamicObjectStorage self, Object key, ThreadState state,
164-
@Shared("readNodeGetItem") @Cached ReadAttributeFromDynamicObjectNode readNode,
164+
@Shared("readKey") @Cached ReadAttributeFromDynamicObjectNode readKey,
165165
@Exclusive @Cached("self.store.getShape()") Shape cachedShape,
166166
@Cached(value = "cachedShape.getKeyList().toArray()", dimensions = 1) Object[] keyList,
167167
@Shared("builtinStringProfile") @Cached IsBuiltinClassProfile profile,
@@ -174,12 +174,12 @@ static Object notString(DynamicObjectStorage self, Object key, ThreadState state
174174
if (gotState.profile(state != null)) {
175175
long keyHash = lib.hashWithState(currentKey, state);
176176
if (keyHash == hash && lib.equalsWithState(key, currentKey, lib, state)) {
177-
return string(self, (String) currentKey, state, readNode);
177+
return string(self, (String) currentKey, state, readKey);
178178
}
179179
} else {
180180
long keyHash = lib.hash(currentKey);
181181
if (keyHash == hash && lib.equals(key, currentKey, lib)) {
182-
return string(self, (String) currentKey, null, readNode);
182+
return string(self, (String) currentKey, null, readKey);
183183
}
184184
}
185185
}
@@ -189,7 +189,7 @@ static Object notString(DynamicObjectStorage self, Object key, ThreadState state
189189

190190
@Specialization(guards = "!isBuiltinString(key, profile)", replaces = "notString", limit = "1")
191191
static Object notStringLoop(DynamicObjectStorage self, Object key, ThreadState state,
192-
@Shared("readNodeGetItem") @Cached ReadAttributeFromDynamicObjectNode readNode,
192+
@Shared("readKey") @Cached ReadAttributeFromDynamicObjectNode readKey,
193193
@Shared("builtinStringProfile") @Cached IsBuiltinClassProfile profile,
194194
@CachedLibrary(limit = "2") PythonObjectLibrary lib,
195195
@Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
@@ -202,12 +202,12 @@ static Object notStringLoop(DynamicObjectStorage self, Object key, ThreadState s
202202
if (gotState.profile(state != null)) {
203203
keyHash = lib.hashWithState(currentKey, state);
204204
if (keyHash == hash && lib.equalsWithState(key, currentKey, lib, state)) {
205-
return string(self, (String) currentKey, state, readNode);
205+
return string(self, (String) currentKey, state, readKey);
206206
}
207207
} else {
208208
keyHash = lib.hash(currentKey);
209209
if (keyHash == hash && lib.equals(key, currentKey, lib)) {
210-
return string(self, (String) currentKey, null, readNode);
210+
return string(self, (String) currentKey, null, readKey);
211211
}
212212
}
213213
}

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,22 @@ public HashingStorage addAllToOther(HashingStorage other,
146146

147147
@ExportMessage
148148
public boolean equalsWithState(HashingStorage other, ThreadState state,
149-
@CachedLibrary(limit = "2") HashingStorageLibrary lib) {
149+
@CachedLibrary(limit = "2") HashingStorageLibrary lib,
150+
@Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
150151
if (this == other) {
151152
return true;
152-
} else if (lib.lengthWithState(this, state) == lib.length(other)) {
153-
return lib.compareEntriesWithState(this, other, state) == 0;
153+
}
154+
if (gotState.profile(state != null)) {
155+
if (lib.lengthWithState(this, state) == lib.lengthWithState(other, state)) {
156+
return lib.compareEntriesWithState(this, other, state) == 0;
157+
}
154158
} else {
155-
return false;
159+
if (lib.length(this) == lib.length(other)) {
160+
return lib.compareEntries(this, other) == 0;
161+
}
156162
}
163+
return false;
164+
157165
}
158166

159167
@GenerateUncached
@@ -225,12 +233,19 @@ HashingStorage[] doit(HashingStorage[] accumulator, Object key,
225233
@ExportMessage
226234
public int compareEntriesWithState(HashingStorage other, ThreadState state,
227235
@CachedLibrary(limit = "2") HashingStorageLibrary lib,
228-
@Cached TestKeyValueEqual testNode) {
236+
@Cached TestKeyValueEqual testNode,
237+
@Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
229238
if (this == other) {
230239
return 0;
231240
}
232-
int otherLen = lib.lengthWithState(other, state);
233-
int selfLen = lib.lengthWithState(this, state);
241+
int otherLen, selfLen;
242+
if (gotState.profile(state != null)) {
243+
otherLen = lib.lengthWithState(other, state);
244+
selfLen = lib.lengthWithState(this, state);
245+
} else {
246+
otherLen = lib.length(other);
247+
selfLen = lib.length(this);
248+
}
234249
if (selfLen > otherLen) {
235250
return 1;
236251
}

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

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@
4343
import java.util.Iterator;
4444

4545
import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
46-
import com.oracle.graal.python.nodes.PRaiseNode;
4746
import com.oracle.truffle.api.library.GenerateLibrary;
47+
import com.oracle.truffle.api.library.GenerateLibrary.DefaultExport;
4848
import com.oracle.truffle.api.library.Library;
4949
import com.oracle.truffle.api.library.LibraryFactory;
5050
import com.oracle.truffle.api.nodes.Node;
@@ -185,9 +185,7 @@ public abstract static class InjectIntoNode extends Node {
185185
*
186186
* @return the return value of the last call to {@link InjectIntoNode#execute}
187187
*/
188-
public HashingStorage[] injectInto(HashingStorage self, HashingStorage[] firstValue, InjectIntoNode node) {
189-
throw new AbstractMethodError("HashingStorageLibrary.injectInto");
190-
}
188+
public abstract HashingStorage[] injectInto(HashingStorage self, HashingStorage[] firstValue, InjectIntoNode node);
191189

192190
/**
193191
* Stores all key-value pairs from {@code self} into {@code other}, replacing key-value pairs
@@ -199,9 +197,7 @@ public HashingStorage[] injectInto(HashingStorage self, HashingStorage[] firstVa
199197
* @return the new store to use for {@code other} from now on, the previous {@code other} has
200198
* become invalid.
201199
*/
202-
public HashingStorage addAllToOther(HashingStorage self, HashingStorage other) {
203-
throw new AbstractMethodError("HashingStorageLibrary.addAllToOther");
204-
}
200+
public abstract HashingStorage addAllToOther(HashingStorage self, HashingStorage other);
205201

206202
/**
207203
* @return an empty store to use from now on, {@code self} has become invalid.
@@ -211,9 +207,7 @@ public HashingStorage addAllToOther(HashingStorage self, HashingStorage other) {
211207
/**
212208
* @return a copy of this store, potentially with a different storage.
213209
*/
214-
public HashingStorage copy(HashingStorage self) {
215-
throw new AbstractMethodError("HashingStorageLibrary.copy");
216-
}
210+
public abstract HashingStorage copy(HashingStorage self);
217211

218212
/**
219213
* @return {@code true} if the key-value pairs are equal between these storages.
@@ -255,37 +249,39 @@ public int compareKeys(HashingStorage self, HashingStorage other) {
255249
* {@code 1} if neither.
256250
*/
257251
public int compareEntriesWithState(HashingStorage self, HashingStorage other, ThreadState state) {
258-
throw new AbstractMethodError("HashingStorageLibrary.compareEntriesWithState");
252+
if (state == null) {
253+
throw new AbstractMethodError("HashingStorageLibrary.compareKeysWithState");
254+
}
255+
return compareEntries(self, other);
259256
}
260257

261258
/**
262-
* @return the intersection of the two storages, keeping the values from {@code other}.
259+
* @see #compareEntriesWithState(HashingStorage, HashingStorage, ThreadState)
263260
*/
264-
public HashingStorage intersect(HashingStorage self, HashingStorage other) {
265-
throw new AbstractMethodError("HashingStorageLibrary.intersect");
261+
public int compareEntries(HashingStorage self, HashingStorage other) {
262+
return compareEntriesWithState(self, other, null);
266263
}
267264

265+
/**
266+
* @return the intersection of the two storages, keeping the values from {@code other}.
267+
*/
268+
public abstract HashingStorage intersect(HashingStorage self, HashingStorage other);
269+
268270
/**
269271
* @return the xor of the two storages.
270272
*/
271-
public HashingStorage xor(HashingStorage self, HashingStorage other) {
272-
throw new AbstractMethodError("HashingStorageLibrary.xor");
273-
}
273+
public abstract HashingStorage xor(HashingStorage self, HashingStorage other);
274274

275275
/**
276276
* @return the union of the two storages, by keys, keeping the values from {@code other} in case
277277
* of conflicts.
278278
*/
279-
public HashingStorage union(HashingStorage self, HashingStorage other) {
280-
throw new AbstractMethodError("HashingStorageLibrary.union");
281-
}
279+
public abstract HashingStorage union(HashingStorage self, HashingStorage other);
282280

283281
/**
284282
* @return the a storage with all keys of {@code self} that are not also in {@code other}
285283
*/
286-
public HashingStorage diff(HashingStorage self, HashingStorage other) {
287-
throw new AbstractMethodError("HashingStorageLibrary.diff");
288-
}
284+
public abstract HashingStorage diff(HashingStorage self, HashingStorage other);
289285

290286
/**
291287
* This method can be used to iterate over the keys of a store. Due to the nature of Java
@@ -294,9 +290,7 @@ public HashingStorage diff(HashingStorage self, HashingStorage other) {
294290
*
295291
* @return an iterator over the keys in this store.
296292
*/
297-
public Iterator<Object> keys(HashingStorage self) {
298-
throw new AbstractMethodError("HashingStorageLibrary.keys");
299-
}
293+
public abstract Iterator<Object> keys(HashingStorage self);
300294

301295
/**
302296
* This method can be used to iterate over the values of a store. Due to the nature of Java

0 commit comments

Comments
 (0)