Skip to content

Commit 73a9cec

Browse files
steve-stimfel
steve-s
authored and
timfel
committed
Add fast path variant for LookupAttributeInMRONode
1 parent fd2f6c0 commit 73a9cec

4 files changed

Lines changed: 100 additions & 21 deletions

File tree

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,20 @@ def __eq__(self, other):
182182
eq_called.append(1)
183183
X.__bases__ = (Base2,)
184184

185+
class Descr:
186+
def __init__(self, value):
187+
self.value = value
188+
def __get__(self, obj, owner=None):
189+
return self.value
190+
def __set__(self, obj, value):
191+
pass
192+
185193
class Base(object):
186-
mykey = 'base 42'
194+
mykey = Descr('base 42')
187195
def __str__(self): return 'Base'
188196

189197
class Base2(object):
190-
mykey = 'base2 42'
198+
mykey = Descr('base2 42')
191199
def __str__(self): return 'Base2'
192200

193201
X = type('X', (Base,), {MyKey(): 5})
@@ -202,6 +210,8 @@ def __str__(self): return 'Base2'
202210
eq_called = []
203211
X = type('X', (Base,), {MyKey(): 5})
204212
xobj = X()
213+
xobj_dict = object.__getattribute__(xobj, "__dict__")
214+
xobj_dict['mykey'] = 'false lead'
205215
assert str(xobj) == 'Base'
206216
assert xobj.mykey == 'base 42'
207217
assert eq_called == [1]

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

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,14 @@
4444
import com.oracle.graal.python.builtins.Python3Core;
4545
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4646
import com.oracle.graal.python.builtins.objects.PNone;
47+
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4748
import com.oracle.graal.python.builtins.objects.type.MroShape;
4849
import com.oracle.graal.python.builtins.objects.type.MroShape.MroShapeLookupResult;
4950
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
5051
import com.oracle.graal.python.builtins.objects.type.PythonClass;
5152
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetMroStorageNode;
5253
import com.oracle.graal.python.builtins.objects.type.TypeNodes.IsSameTypeNode;
54+
import com.oracle.graal.python.nodes.PGuards;
5355
import com.oracle.graal.python.nodes.PNodeWithContext;
5456
import com.oracle.graal.python.runtime.PythonContext;
5557
import com.oracle.graal.python.runtime.PythonOptions;
@@ -64,6 +66,7 @@
6466
import com.oracle.truffle.api.dsl.Bind;
6567
import com.oracle.truffle.api.dsl.Cached;
6668
import com.oracle.truffle.api.dsl.Cached.Shared;
69+
import com.oracle.truffle.api.dsl.GenerateCached;
6770
import com.oracle.truffle.api.dsl.GenerateInline;
6871
import com.oracle.truffle.api.dsl.GenerateUncached;
6972
import com.oracle.truffle.api.dsl.Idempotent;
@@ -75,6 +78,7 @@
7578
import com.oracle.truffle.api.nodes.ExplodeLoop;
7679
import com.oracle.truffle.api.nodes.ExplodeLoop.LoopExplosionKind;
7780
import com.oracle.truffle.api.nodes.Node;
81+
import com.oracle.truffle.api.object.DynamicObject;
7882
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
7983
import com.oracle.truffle.api.strings.TruffleString;
8084

@@ -156,6 +160,7 @@ public static LookupAttributeInMRONode createForLookupOfUnmanagedClasses(Truffle
156160
return LookupAttributeInMRONodeGen.create(key, true);
157161
}
158162

163+
@NeverDefault
159164
static Object findAttr(Python3Core core, PythonBuiltinClassType klass, TruffleString key) {
160165
return findAttr(core, klass, key, ReadAttributeFromPythonObjectNode.getUncached());
161166
}
@@ -202,7 +207,22 @@ public MROChangedException(Object result) {
202207
}
203208
}
204209

210+
@SuppressWarnings("serial")
211+
public static class MROGenericDictException extends StacktracelessCheckedException {
212+
private static final MROGenericDictException INSTANCE = new MROGenericDictException();
213+
}
214+
205215
MroSequenceStorage.FinalAttributeAssumptionPair findAttrAndAssumptionInMRO(Object klass) throws MROChangedException {
216+
try {
217+
return findAttrAndAssumptionInMRO(this, klass, key, skipNonStaticBases, false);
218+
} catch (MROGenericDictException ignore) {
219+
throw CompilerDirectives.shouldNotReachHere();
220+
}
221+
}
222+
223+
@NeverDefault
224+
static MroSequenceStorage.FinalAttributeAssumptionPair findAttrAndAssumptionInMRO(Node n, Object klass, TruffleString key, boolean skipNonStaticBases,
225+
boolean mustNotSideEffect) throws MROChangedException, MROGenericDictException {
206226
CompilerAsserts.neverPartOfCompilation();
207227
// Regarding potential side effects to MRO caused by __eq__ of the keys in the dicts that we
208228
// search through: CPython seems to read the MRO once and then compute the result also
@@ -214,19 +234,30 @@ MroSequenceStorage.FinalAttributeAssumptionPair findAttrAndAssumptionInMRO(Objec
214234
if (assumptionNode != null) {
215235
return assumptionNode;
216236
}
217-
// Put a new assumption in place in case the MRO changes during the lookup
218237
MroSequenceStorage.FinalAttributeAssumptionPair assumptionPair = new MroSequenceStorage.FinalAttributeAssumptionPair();
219-
mro.putFinalAttributeAssumption(key, assumptionPair);
238+
if (!mustNotSideEffect) {
239+
// Put a new assumption in place in case the MRO changes during the lookup
240+
mro.putFinalAttributeAssumption(key, assumptionPair);
241+
}
220242
EncapsulatingNodeReference nodeRef = EncapsulatingNodeReference.getCurrent();
221-
Node prev = nodeRef.set(this);
243+
Node prev = nodeRef.set(n);
222244
Object result = PNone.NO_VALUE;
223245
try {
224246
for (int i = 0; i < mro.length(); i++) {
225247
PythonAbstractClass clsObj = mro.getPythonClassItemNormalized(i);
226248
if (skipNonStaticBase(clsObj, skipNonStaticBases)) {
227249
continue;
228250
}
229-
Object value = ReadAttributeFromObjectNode.getUncached().execute(clsObj, key);
251+
Object value;
252+
if (mustNotSideEffect) {
253+
if (clsObj instanceof PythonObject pyClsObj && !PGuards.hasMaterializedDict(pyClsObj.getShape())) {
254+
value = DynamicObject.GetNode.getUncached().execute(pyClsObj, key, PNone.NO_VALUE);
255+
} else {
256+
throw MROGenericDictException.INSTANCE;
257+
}
258+
} else {
259+
value = ReadAttributeFromObjectNode.getUncached().execute(clsObj, key);
260+
}
230261
if (value != PNone.NO_VALUE) {
231262
result = value;
232263
break;
@@ -240,6 +271,10 @@ MroSequenceStorage.FinalAttributeAssumptionPair findAttrAndAssumptionInMRO(Objec
240271
// exception. This should abort the specialization
241272
throw new MROChangedException(result);
242273
}
274+
if (mustNotSideEffect) {
275+
// must connect the assumption with the MRO here, otherwise we may end up with half initialized FinalAttributeAssumptionPair if we had to bail out because we found a generic dict
276+
mro.putFinalAttributeAssumption(key, assumptionPair);
277+
}
243278
assumptionPair.setValue(result);
244279
return assumptionPair;
245280
}
@@ -264,6 +299,45 @@ Object lookupSlowPath(Object klass,
264299
return slowPathNode.execute(klass, key, skipNonStaticBases);
265300
}
266301

302+
@GenerateInline
303+
@GenerateCached(false)
304+
@ImportStatic(LookupAttributeInMRONode.class)
305+
public abstract static class CachedKeyFastPath extends PNodeWithContext {
306+
307+
public final Object execute(Node inliningTarget, Object klass, TruffleString key) {
308+
CompilerAsserts.partialEvaluationConstant(klass);
309+
CompilerAsserts.partialEvaluationConstant(key);
310+
try {
311+
return executeImpl(inliningTarget, klass, key);
312+
} catch (MROChangedException e) {
313+
throw CompilerDirectives.shouldNotReachHere();
314+
} catch (MROGenericDictException e) {
315+
CompilerDirectives.transferToInterpreterAndInvalidate();
316+
return null;
317+
}
318+
}
319+
320+
public abstract Object executeImpl(Node inliningTarget, Object klass, TruffleString key) throws MROChangedException, MROGenericDictException;
321+
322+
@Specialization
323+
static Object lookupPBCTCached(Node inliningTarget, PythonBuiltinClassType klass, TruffleString key,
324+
@Bind PythonContext context,
325+
@Cached("findAttr(context, klass, key)") Object cachedValue) {
326+
return cachedValue;
327+
}
328+
329+
@Specialization(assumptions = "cachedAttrInMROInfo.getAssumption()", guards = "isSingleContext()")
330+
static Object lookupConstantMROCached(Node inliningTarget, Object klass, TruffleString key,
331+
@Cached("findAttrAndAssumptionInMRO(inliningTarget, klass, key, false, true)") MroSequenceStorage.FinalAttributeAssumptionPair cachedAttrInMROInfo) {
332+
return cachedAttrInMROInfo.getValue();
333+
}
334+
335+
@Specialization(replaces = {"lookupPBCTCached", "lookupConstantMROCached"})
336+
static Object noFastPath(Object klass, TruffleString key) {
337+
return null;
338+
}
339+
}
340+
267341
public abstract static class SlowPath extends PNodeWithContext {
268342

269343
@Child private GetMroStorageNode getMroNode;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,10 +2012,10 @@ static Object doType(VirtualFrame frame, TruffleString key, PythonManagedClass r
20122012

20132013
// The convention is that if klass is null, then the object is assumed to be of a builtin type that has the object's or module's tp_getattro - we do not need to recheck that dynamically
20142014
public static Object loadInstanceValue(Node inliningTarget, PythonObject object, PythonManagedClass klass,
2015-
LookupAttributeInMRONode getDesc, Shape cachedShape, PropertyGetter cachedPropertyGetter,
2015+
TruffleString key, LookupAttributeInMRONode.CachedKeyFastPath getDesc, Shape cachedShape, PropertyGetter cachedPropertyGetter,
20162016
InlineWeakValueProfile slotsValueProfile) {
20172017
if (klass == null || hasObjectOrModuleGetattro(inliningTarget, klass, slotsValueProfile)) {
2018-
Object descr = getDesc.execute(cachedShape.getDynamicType());
2018+
Object descr = getDesc.execute(inliningTarget, cachedShape.getDynamicType(), key);
20192019
if (descr == PNone.NO_VALUE) {
20202020
assert object.checkDictFlags();
20212021
Object value = cachedPropertyGetter.get(object);
@@ -2054,9 +2054,9 @@ static Object doInstanceValue(TruffleString key, PythonObject receiver,
20542054
@Cached("receiver.getShape()") Shape cachedShape,
20552055
@Cached("getManagedClassOrNull(cachedShape)") PythonManagedClass managedClass,
20562056
@Cached("getPropertyGetterWithFinalAssumption(cachedShape, key)") PropertyGetter getter,
2057-
@Cached("create(key)") LookupAttributeInMRONode getDesc,
2057+
@Cached LookupAttributeInMRONode.CachedKeyFastPath getDesc,
20582058
@Cached InlineWeakValueProfile slotsValueProfile,
2059-
@Bind("loadInstanceValue(inliningTarget, receiver, managedClass, getDesc, cachedShape, getter, slotsValueProfile)") Object value) {
2059+
@Bind("loadInstanceValue(inliningTarget, receiver, managedClass, key, getDesc, cachedShape, getter, slotsValueProfile)") Object value) {
20602060
return value;
20612061
}
20622062

@@ -2084,11 +2084,11 @@ public static Object doItUncached(VirtualFrame frame, TruffleString key, Object
20842084
@ImportStatic(PGuards.class)
20852085
public static final class SetAttribute {
20862086
@NonIdempotent
2087-
public static boolean canStoreInstanceValue(Node inliningTarget, PythonManagedClass managedClass, Shape cachedShape, LookupAttributeInMRONode getDesc,
2087+
public static boolean canStoreInstanceValue(Node inliningTarget, TruffleString key, PythonManagedClass managedClass, Shape cachedShape, LookupAttributeInMRONode.CachedKeyFastPath getDesc,
20882088
GetObjectSlotsNode getDescSlotsNode, InlineWeakValueProfile slotsValueProfile) {
20892089
if (managedClass == null || slotsValueProfile.execute(inliningTarget, managedClass.getTpSlots()).tp_setattro() == ObjectBuiltins.SLOTS.tp_setattro()) {
2090-
Object descr = getDesc.execute(cachedShape.getDynamicType());
2091-
return descr == PNone.NO_VALUE || getDescSlotsNode.execute(inliningTarget, descr).tp_descr_set() == null;
2090+
Object descr = getDesc.execute(inliningTarget, cachedShape.getDynamicType(), key);
2091+
return descr != null && (descr == PNone.NO_VALUE || getDescSlotsNode.execute(inliningTarget, descr).tp_descr_set() == null);
20922092
}
20932093
return false;
20942094
}
@@ -2123,13 +2123,14 @@ public static boolean hasNoSlotsOrMaterializedDict(Shape cachedShape) {
21232123
@Specialization(guards = {
21242124
"hasNoSlotsOrMaterializedDict(cachedShape)", "managedClass != null || isBuiltinWithObjectSetattro(cachedShape)", //
21252125
"cachedShape.check(receiver)", //
2126-
"skipDescriptorCheck || canStoreInstanceValue(inliningTarget, managedClass, cachedShape, getDesc, getDescSlotsNode, slotsValueProfile)"}, limit = "3")
2126+
"skipDescriptorCheck || canStoreInstanceValue(inliningTarget, cachedKey, managedClass, cachedShape, getDesc, getDescSlotsNode, slotsValueProfile)"}, limit = "3")
21272127
static void doInstanceValue(TruffleString key, Object value, PythonObject receiver,
21282128
@Bind Node inliningTarget,
2129+
@Cached("key") TruffleString cachedKey,
21292130
@Cached("receiver.getShape()") Shape cachedShape,
21302131
@Cached("getManagedClassOrNull(cachedShape)") PythonManagedClass managedClass,
21312132
@Cached("canSkipDescriptorCheck(cachedShape, key)") boolean skipDescriptorCheck,
2132-
@Cached("create(key)") LookupAttributeInMRONode getDesc,
2133+
@Cached LookupAttributeInMRONode.CachedKeyFastPath getDesc,
21332134
@Cached GetObjectSlotsNode getDescSlotsNode,
21342135
@Cached InlineWeakValueProfile slotsValueProfile,
21352136
@Cached DynamicObject.PutNode putNode) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadBuiltinNode.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import com.oracle.truffle.api.dsl.NonIdempotent;
6565
import com.oracle.truffle.api.dsl.Specialization;
6666
import com.oracle.truffle.api.nodes.Node;
67-
import com.oracle.truffle.api.object.DynamicObject.GetNode;
6867
import com.oracle.truffle.api.object.PropertyGetter;
6968
import com.oracle.truffle.api.object.Shape;
7069
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
@@ -76,11 +75,6 @@
7675
public abstract class ReadBuiltinNode extends PNodeWithContext {
7776
public abstract Object execute(TruffleString attributeId);
7877

79-
@NeverDefault
80-
static Object readAttribute(DynamicObjectStorage domStorage, TruffleString name) {
81-
return GetNode.getUncached().execute(domStorage.getStore(), name, PNone.NO_VALUE);
82-
}
83-
8478
@NonIdempotent
8579
static boolean getterAccepts(PropertyGetter getter, PythonModule mod) {
8680
return getter.accepts(mod);

0 commit comments

Comments
 (0)