Skip to content

Commit 0111cdf

Browse files
timfeltimfel
committed
[GR-12820] [GR-12709] Fix performance regressions.
PullRequest: graalpython/306
2 parents e0a2cc9 + 64cf870 commit 0111cdf

File tree

13 files changed

+96
-58
lines changed

13 files changed

+96
-58
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_gc.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,7 @@
4242

4343

4444
def test_gc_collect():
45-
class Foo():
46-
pass
47-
x = Foo()
48-
r = ref(x)
49-
assert r() is not None, "weakref should not be collected"
50-
x = None
51-
gc.collect()
52-
gc.collect() # to be safe :(
53-
assert r() is None, "weakref should have been collected"
45+
assert isinstance(gc.collect(), int)
5446

5547

5648
def test_gc_count():

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.oracle.graal.python.builtins.Builtin;
3333
import com.oracle.graal.python.builtins.CoreFunctions;
3434
import com.oracle.graal.python.builtins.PythonBuiltins;
35-
import com.oracle.graal.python.builtins.objects.PNone;
3635
import com.oracle.graal.python.builtins.objects.cext.PythonNativeClass;
3736
import com.oracle.graal.python.builtins.objects.cext.PythonNativeObject;
3837
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
@@ -55,9 +54,9 @@ protected List<com.oracle.truffle.api.dsl.NodeFactory<? extends PythonBuiltinNod
5554
abstract static class GcCollectNode extends PythonBuiltinNode {
5655
@Specialization
5756
@TruffleBoundary
58-
Object collect() {
57+
int collect() {
5958
System.gc();
60-
return PNone.NONE;
59+
return 0;
6160
}
6261
}
6362

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ private void ensureCapiWasLoaded() {
240240
throw raise(PythonErrorType.ImportError, "cannot load capi from " + capiFile.getAbsoluteFile().getPath());
241241
}
242242
// call into Python to initialize python_cext module globals
243-
ReadAttributeFromObjectNode readNode = ReadAttributeFromObjectNode.create();
243+
ReadAttributeFromObjectNode readNode = insert(ReadAttributeFromObjectNode.create());
244244
CallUnaryMethodNode callNode = insert(CallUnaryMethodNode.create());
245245
callNode.executeObject(readNode.execute(ctxt.getCore().lookupBuiltinModule("python_cext"), INITIALIZE_CAPI), capi);
246246
ctxt.setCapiWasLoaded(capi);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1723,7 +1723,7 @@ Object upcall(VirtualFrame frame, PythonModule cextModule, Object receiver, Stri
17231723
} catch (PException e) {
17241724
if (readErrorHandlerNode == null) {
17251725
CompilerDirectives.transferToInterpreterAndInvalidate();
1726-
readErrorHandlerNode = ReadAttributeFromObjectNode.create();
1726+
readErrorHandlerNode = insert(ReadAttributeFromObjectNode.create());
17271727
}
17281728
getContext().setCurrentException(e);
17291729
return toSulongNode.execute(readErrorHandlerNode.execute(cextModule, NATIVE_NULL));

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PNodeWithContext.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,13 @@
5151
import com.oracle.graal.python.runtime.exception.PythonErrorType;
5252
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
5353
import com.oracle.truffle.api.Assumption;
54+
import com.oracle.truffle.api.CompilerAsserts;
5455
import com.oracle.truffle.api.CompilerDirectives;
5556
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
5657
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5758
import com.oracle.truffle.api.TruffleLanguage.ContextReference;
5859
import com.oracle.truffle.api.nodes.Node;
60+
import com.oracle.truffle.api.nodes.RootNode;
5961
import com.oracle.truffle.api.profiles.ConditionProfile;
6062

6163
public abstract class PNodeWithContext extends Node {
@@ -121,7 +123,14 @@ public final PythonContext getContext() {
121123
}
122124

123125
protected Assumption singleContextAssumption() {
124-
PythonLanguage language = getRootNode().getLanguage(PythonLanguage.class);
126+
CompilerAsserts.neverPartOfCompilation("the singleContextAssumption should only be retrieved in the interpreter");
127+
PythonLanguage language = null;
128+
RootNode rootNode = getRootNode();
129+
if (rootNode != null) {
130+
language = rootNode.getLanguage(PythonLanguage.class);
131+
} else {
132+
throw new IllegalStateException("a python node was executed without being adopted!");
133+
}
125134
if (language == null) {
126135
language = PythonLanguage.getCurrent();
127136
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ObjectAttributeNode.java

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@
4040
*/
4141
package com.oracle.graal.python.nodes.attributes;
4242

43-
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage;
4443
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
44+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.GetDictStorageNode;
45+
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
4546
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
4647
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4748
import com.oracle.graal.python.builtins.objects.str.PString;
@@ -50,21 +51,14 @@
5051
import com.oracle.graal.python.runtime.PythonOptions;
5152
import com.oracle.truffle.api.CompilerDirectives;
5253
import com.oracle.truffle.api.dsl.ImportStatic;
54+
import com.oracle.truffle.api.nodes.NodeCost;
5355
import com.oracle.truffle.api.object.HiddenKey;
5456
import com.oracle.truffle.api.object.Location;
5557
import com.oracle.truffle.api.object.Property;
5658

5759
@ImportStatic({PGuards.class, PythonOptions.class})
5860
public abstract class ObjectAttributeNode extends PNodeWithContext {
59-
private @Child HashingCollectionNodes.GetDictStorageNode getDictStorageNode;
60-
61-
public HashingCollectionNodes.GetDictStorageNode getGetDictStorageNode() {
62-
if (getDictStorageNode == null) {
63-
CompilerDirectives.transferToInterpreterAndInvalidate();
64-
getDictStorageNode = insert(HashingCollectionNodes.GetDictStorageNode.create());
65-
}
66-
return getDictStorageNode;
67-
}
61+
@Child private GetDictStorageNode getStorageNode;
6862

6963
protected Object attrKey(Object key) {
7064
if (key instanceof PString) {
@@ -74,13 +68,16 @@ protected Object attrKey(Object key) {
7468
}
7569
}
7670

77-
protected boolean isDictUnsetOrSameAsStorage(PythonObject object) {
78-
PHashingCollection dict = object.getDict();
79-
if (dict == null) {
80-
return true;
81-
} else {
82-
return getGetDictStorageNode().execute(dict) instanceof DynamicObjectStorage.PythonObjectDictStorage;
71+
protected HashingStorage getDictStorage(PHashingCollection c) {
72+
if (getStorageNode == null) {
73+
CompilerDirectives.transferToInterpreterAndInvalidate();
74+
getStorageNode = insert(HashingCollectionNodes.GetDictStorageNode.create());
8375
}
76+
return getStorageNode.execute(c);
77+
}
78+
79+
protected boolean isDictUnsetOrSameAsStorage(PythonObject object) {
80+
return object.getDict() == null;
8481
}
8582

8683
protected Location getLocationOrNull(Property prop) {
@@ -90,4 +87,10 @@ protected Location getLocationOrNull(Property prop) {
9087
protected static boolean isHiddenKey(Object key) {
9188
return key instanceof HiddenKey;
9289
}
90+
91+
@Override
92+
public NodeCost getCost() {
93+
// really just a few guards and delegation
94+
return NodeCost.NONE;
95+
}
9396
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromDynamicObjectNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ private static boolean assertFinal(DynamicObject dynamicObject, Object key, Obje
8787
"loc.isAssumedFinal()",
8888
}, //
8989
assumptions = {
90+
"singleContextAssumption",
9091
"layoutAssumption",
9192
"finalAssumption"
9293
})
@@ -98,6 +99,7 @@ protected Object readDirectFinal(DynamicObject dynamicObject, Object key,
9899
@Cached("cachedShape.getValidAssumption()") Assumption layoutAssumption,
99100
@Cached("getLocationOrNull(cachedShape.getProperty(attrKey))") Location loc,
100101
@Cached("loc.getFinalAssumption()") Assumption finalAssumption,
102+
@SuppressWarnings("unused") @Cached("singleContextAssumption()") Assumption singleContextAssumption,
101103
@Cached("readFinalValue(dynamicObject, loc)") Object cachedValue) {
102104
assert assertFinal(dynamicObject, attrKey, cachedValue);
103105
return cachedValue;
@@ -108,7 +110,6 @@ protected Object readDirectFinal(DynamicObject dynamicObject, Object key,
108110
guards = {
109111
"dynamicObject.getShape() == cachedShape",
110112
"key == cachedKey",
111-
"loc == null || !loc.isAssumedFinal()",
112113
}, //
113114
assumptions = {
114115
"layoutAssumption"

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromObjectNode.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.GetObjectDictNode;
4545
import com.oracle.graal.python.builtins.objects.cext.PythonNativeObject;
4646
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
47-
import com.oracle.graal.python.builtins.objects.dict.PDict;
47+
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
4848
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4949
import com.oracle.graal.python.nodes.PGuards;
5050
import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode;
@@ -72,10 +72,12 @@ public static ReadAttributeFromObjectNode create() {
7272
@Specialization(guards = {
7373
"object == cachedObject"
7474
}, assumptions = {
75+
"singleContextAssumption",
7576
"dictUnsetOrSameAsStorageAssumption"
7677
})
7778
protected Object readFromDynamicStorageCached(PythonObject object, Object key,
7879
@SuppressWarnings("unused") @Cached("object") PythonObject cachedObject,
80+
@SuppressWarnings("unused") @Cached("singleContextAssumption()") Assumption singleContextAssumption,
7981
@SuppressWarnings("unused") @Cached("cachedObject.getDictUnsetOrSameAsStorageAssumption()") Assumption dictUnsetOrSameAsStorageAssumption,
8082
@Cached("create()") ReadAttributeFromDynamicObjectNode readAttributeFromDynamicObjectNode) {
8183
return readAttributeFromDynamicObjectNode.execute(object.getStorage(), key);
@@ -93,12 +95,15 @@ protected Object readFromDynamicStorage(PythonObject object, Object key,
9395
@Specialization(guards = {
9496
"object == cachedObject",
9597
"!dictUnsetOrSameAsStorageAssumption.isValid()"
98+
}, assumptions = {
99+
"singleContextAssumption"
96100
})
97101
protected Object readFromDictCached(PythonObject object, Object key,
98102
@SuppressWarnings("unused") @Cached("object") PythonObject cachedObject,
103+
@SuppressWarnings("unused") @Cached("singleContextAssumption()") Assumption singleContextAssumption,
99104
@SuppressWarnings("unused") @Cached("cachedObject.getDictUnsetOrSameAsStorageAssumption()") Assumption dictUnsetOrSameAsStorageAssumption,
100105
@Cached("create()") HashingStorageNodes.GetItemNode getItemNode) {
101-
Object value = getItemNode.execute(object.getDict().getDictStorage(), key);
106+
Object value = getItemNode.execute(getDictStorage(object.getDict()), key);
102107
if (value == null) {
103108
return PNone.NO_VALUE;
104109
} else {
@@ -111,7 +116,7 @@ protected Object readFromDictCached(PythonObject object, Object key,
111116
}, replaces = "readFromDictCached")
112117
protected Object readFromDict(PythonObject object, Object key,
113118
@Cached("create()") HashingStorageNodes.GetItemNode getItemNode) {
114-
Object value = getItemNode.execute(object.getDict().getDictStorage(), key);
119+
Object value = getItemNode.execute(getDictStorage(object.getDict()), key);
115120
if (value == null) {
116121
return PNone.NO_VALUE;
117122
} else {
@@ -130,8 +135,8 @@ protected Object readNative(PythonNativeObject object, Object key,
130135
@Cached("create()") HashingStorageNodes.GetItemNode getItemNode) {
131136
Object d = getNativeDict.execute(object);
132137
Object value = null;
133-
if (d instanceof PDict) {
134-
value = getItemNode.execute(((PDict) d).getDictStorage(), key);
138+
if (d instanceof PHashingCollection) {
139+
value = getItemNode.execute(getDictStorage((PHashingCollection) d), key);
135140
}
136141
if (value == null) {
137142
return PNone.NO_VALUE;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/WriteAttributeToObjectNode.java

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@
4343
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4444
import com.oracle.graal.python.builtins.objects.cext.CExtNodes.GetObjectDictNode;
4545
import com.oracle.graal.python.builtins.objects.cext.PythonNativeObject;
46+
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes;
4647
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
4748
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
4849
import com.oracle.graal.python.builtins.objects.common.PHashingCollection;
49-
import com.oracle.graal.python.builtins.objects.dict.PDict;
5050
import com.oracle.graal.python.builtins.objects.exception.PBaseException;
5151
import com.oracle.graal.python.builtins.objects.function.PFunction;
5252
import com.oracle.graal.python.builtins.objects.method.PMethod;
@@ -60,6 +60,7 @@
6060
import com.oracle.truffle.api.dsl.Fallback;
6161
import com.oracle.truffle.api.dsl.ImportStatic;
6262
import com.oracle.truffle.api.dsl.Specialization;
63+
import com.oracle.truffle.api.profiles.BranchProfile;
6364
import com.oracle.truffle.api.profiles.ConditionProfile;
6465

6566
@ImportStatic(PythonOptions.class)
@@ -93,13 +94,15 @@ private void handlePythonClass(PythonObject object, Object key) {
9394

9495
// write to the DynamicObject
9596
@Specialization(guards = {
96-
"isAttrWritable(object) || isHiddenKey(key)",
97-
"object == cachedObject"
97+
"object == cachedObject",
98+
"isAttrWritable(object) || isHiddenKey(key)"
9899
}, assumptions = {
100+
"singleContextAssumption",
99101
"dictUnsetOrSameAsStorageAssumption"
100102
})
101103
protected boolean writeToDynamicStorageCached(PythonObject object, Object key, Object value,
102104
@SuppressWarnings("unused") @Cached("object") PythonObject cachedObject,
105+
@SuppressWarnings("unused") @Cached("singleContextAssumption()") Assumption singleContextAssumption,
103106
@SuppressWarnings("unused") @Cached("cachedObject.getDictUnsetOrSameAsStorageAssumption()") Assumption dictUnsetOrSameAsStorageAssumption,
104107
@Cached("create()") WriteAttributeToDynamicObjectNode writeAttributeToDynamicObjectNode) {
105108
handlePythonClass(object, key);
@@ -120,37 +123,50 @@ protected boolean writeToDynamicStorage(PythonObject object, Object key, Object
120123
@Specialization(guards = {
121124
"object == cachedObject",
122125
"!dictUnsetOrSameAsStorageAssumption.isValid()"
126+
}, assumptions = {
127+
"singleContextAssumption"
123128
})
124129
protected boolean writeToDictCached(PythonObject object, Object key, Object value,
125130
@SuppressWarnings("unused") @Cached("object") PythonObject cachedObject,
131+
@SuppressWarnings("unused") @Cached("singleContextAssumption()") Assumption singleContextAssumption,
126132
@SuppressWarnings("unused") @Cached("cachedObject.getDictUnsetOrSameAsStorageAssumption()") Assumption dictUnsetOrSameAsStorageAssumption,
133+
@Cached("create()") BranchProfile updateStorage,
127134
@Cached("create()") HashingStorageNodes.SetItemNode setItemNode) {
128135
handlePythonClass(object, key);
129136
PHashingCollection dict = object.getDict();
130-
HashingStorage hashingStorage = setItemNode.execute(dict.getDictStorage(), key, value);
131-
dict.setDictStorage(hashingStorage);
137+
HashingStorage dictStorage = getDictStorage(dict);
138+
HashingStorage hashingStorage = setItemNode.execute(dictStorage, key, value);
139+
if (dictStorage != hashingStorage) {
140+
updateStorage.enter();
141+
dict.setDictStorage(hashingStorage);
142+
}
132143
return true;
133144
}
134145

135146
@Specialization(guards = {
136147
"!isDictUnsetOrSameAsStorage(object)"
137148
}, replaces = "writeToDictCached")
138149
protected boolean writeToDict(PythonObject object, Object key, Object value,
150+
@Cached("create()") BranchProfile updateStorage,
139151
@Cached("create()") HashingStorageNodes.SetItemNode setItemNode) {
140152
handlePythonClass(object, key);
141153
PHashingCollection dict = object.getDict();
142-
HashingStorage hashingStorage = setItemNode.execute(dict.getDictStorage(), key, value);
143-
dict.setDictStorage(hashingStorage);
154+
HashingStorage dictStorage = getDictStorage(dict);
155+
HashingStorage hashingStorage = setItemNode.execute(dictStorage, key, value);
156+
if (dictStorage != hashingStorage) {
157+
updateStorage.enter();
158+
dict.setDictStorage(hashingStorage);
159+
}
144160
return true;
145161
}
146162

147163
@Specialization(guards = "!isPythonObject(object)")
148164
protected boolean readNative(PythonNativeObject object, Object key, Object value,
149165
@Cached("create()") GetObjectDictNode getNativeDict,
150-
@Cached("create()") HashingStorageNodes.SetItemNode setItemNode) {
166+
@Cached("create()") HashingCollectionNodes.SetItemNode setItemNode) {
151167
Object d = getNativeDict.execute(object);
152-
if (d instanceof PDict) {
153-
setItemNode.execute(((PDict) d).getDictStorage(), key, value);
168+
if (d instanceof PHashingCollection) {
169+
setItemNode.execute(((PHashingCollection) d), key, value);
154170
return true;
155171
} else {
156172
return raise(object, key, value);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/builtins/TupleNodes.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
package com.oracle.graal.python.nodes.builtins;
4242

4343
import java.util.ArrayList;
44-
import java.util.List;
4544

4645
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4746
import com.oracle.graal.python.builtins.objects.PNone;
@@ -104,25 +103,39 @@ public PTuple tuple(@SuppressWarnings("unused") LazyPythonClass cls, PTuple iter
104103
return iterable;
105104
}
106105

107-
@TruffleBoundary(allowInlining = true, transferToInterpreterOnException = false)
108106
@Specialization(guards = {"!isNoValue(iterable)", "createNewTuple(cls, iterable)"})
109107
public PTuple tuple(LazyPythonClass cls, Object iterable,
110108
@Cached("create()") GetIteratorNode getIterator,
111109
@Cached("create()") GetNextNode next,
112110
@Cached("create()") IsBuiltinClassProfile errorProfile) {
113111

114112
Object iterator = getIterator.executeWith(iterable);
115-
List<Object> internalStorage = new ArrayList<>();
113+
ArrayList<Object> internalStorage = makeList();
116114
while (true) {
117115
try {
118-
internalStorage.add(next.execute(iterator));
116+
addToList(internalStorage, next.execute(iterator));
119117
} catch (PException e) {
120118
e.expectStopIteration(errorProfile);
121-
return factory().createTuple(cls, internalStorage.toArray());
119+
return factory().createTuple(cls, listToArray(internalStorage));
122120
}
123121
}
124122
}
125123

124+
@TruffleBoundary
125+
private static ArrayList<Object> makeList() {
126+
return new ArrayList<>();
127+
}
128+
129+
@TruffleBoundary
130+
private static void addToList(ArrayList<Object> list, Object obj) {
131+
list.add(obj);
132+
}
133+
134+
@TruffleBoundary
135+
private static Object[] listToArray(ArrayList<Object> list) {
136+
return list.toArray();
137+
}
138+
126139
@Fallback
127140
public PTuple tuple(@SuppressWarnings("unused") LazyPythonClass cls, Object value) {
128141
CompilerDirectives.transferToInterpreter();

0 commit comments

Comments
 (0)