Skip to content

Commit 2a2ef65

Browse files
msimacekmsimacek
committed
[GR-71105] Simplify MRO lookup assumptions
PullRequest: graalpython/4123
2 parents 386c8c6 + bd3e544 commit 2a2ef65

File tree

13 files changed

+215
-304
lines changed

13 files changed

+215
-304
lines changed

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

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
import com.oracle.graal.python.nodes.PGuards;
5858
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromPythonObjectNode;
5959
import com.oracle.graal.python.nodes.util.CastToTruffleStringNode;
60-
import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage;
6160
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
6261
import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff;
6362
import com.oracle.truffle.api.dsl.Bind;
@@ -79,7 +78,6 @@
7978
import com.oracle.truffle.api.object.DynamicObject;
8079
import com.oracle.truffle.api.object.DynamicObjectLibrary;
8180
import com.oracle.truffle.api.object.Shape;
82-
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
8381
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
8482
import com.oracle.truffle.api.strings.TruffleString;
8583

@@ -92,7 +90,6 @@ public final class DynamicObjectStorage extends HashingStorage {
9290
public static final int EXPLODE_LOOP_SIZE_LIMIT = 16;
9391

9492
final DynamicObject store;
95-
private final MroSequenceStorage mro;
9693

9794
static final class Store extends DynamicObject {
9895
public Store(Shape shape) {
@@ -101,20 +98,11 @@ public Store(Shape shape) {
10198
}
10299

103100
public DynamicObjectStorage(PythonLanguage lang) {
104-
this(new Store(lang.getEmptyShape()), null);
101+
this(new Store(lang.getEmptyShape()));
105102
}
106103

107104
public DynamicObjectStorage(DynamicObject store) {
108-
this(store, null);
109-
}
110-
111-
public DynamicObjectStorage(DynamicObject store, MroSequenceStorage mro) {
112105
this.store = store;
113-
this.mro = mro;
114-
}
115-
116-
public Shape getStoreShape() {
117-
return store.getShape();
118106
}
119107

120108
public DynamicObject getStore() {
@@ -289,20 +277,8 @@ private static boolean hasNext(Iterator<Object> keys) {
289277
}
290278
}
291279

292-
private static void invalidateAttributeInMROFinalAssumptions(MroSequenceStorage mro, TruffleString name, Node inliningTarget, InlinedBranchProfile profile) {
293-
if (mro != null) {
294-
profile.enter(inliningTarget);
295-
mro.invalidateAttributeInMROFinalAssumptions(name);
296-
}
297-
}
298-
299-
void setStringKey(TruffleString key, Object value, DynamicObjectLibrary dylib, Node inliningTarget, InlinedBranchProfile invalidateMroProfile) {
280+
void setStringKey(TruffleString key, Object value, DynamicObjectLibrary dylib) {
300281
dylib.put(store, key, assertNoJavaString(value));
301-
invalidateAttributeInMROFinalAssumption(key, inliningTarget, invalidateMroProfile);
302-
}
303-
304-
void invalidateAttributeInMROFinalAssumption(TruffleString key, Node inliningTarget, InlinedBranchProfile invalidateMroProfile) {
305-
invalidateAttributeInMROFinalAssumptions(mro, key, inliningTarget, invalidateMroProfile);
306282
}
307283

308284
boolean shouldTransitionOnPut() {
@@ -393,10 +369,9 @@ public static DynamicObjectStorage copyGeneric(DynamicObjectStorage receiver,
393369
@GenerateCached(false)
394370
public abstract static class DynamicObjectStorageSetStringKey extends SpecializedSetStringKey {
395371
@Specialization
396-
static void doIt(Node inliningTarget, HashingStorage self, TruffleString key, Object value,
397-
@CachedLibrary(limit = "3") DynamicObjectLibrary dylib,
398-
@Cached InlinedBranchProfile invalidateMro) {
399-
((DynamicObjectStorage) self).setStringKey(key, value, dylib, inliningTarget, invalidateMro);
372+
static void doIt(HashingStorage self, TruffleString key, Object value,
373+
@CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
374+
((DynamicObjectStorage) self).setStringKey(key, value, dylib);
400375
}
401376
}
402377
}

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

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ abstract static class SpecializedSetStringKey extends Node {
232232
}
233233

234234
static EconomicMapStorage dynamicObjectStorageToEconomicMap(Node inliningTarget, DynamicObjectStorage s, DynamicObjectLibrary dylib, PyObjectHashNode hashNode, ObjectHashMap.PutNode putNode) {
235-
// TODO: shouldn't we invalidate all MRO assumptions in this case?
236235
DynamicObject store = s.store;
237236
EconomicMapStorage result = EconomicMapStorage.create(dylib.getShape(store).getPropertyCount());
238237
ObjectHashMap resultMap = result.map;
@@ -280,10 +279,9 @@ static HashingStorage empty(Frame frame, Node inliningTarget, @SuppressWarnings(
280279
}
281280

282281
@Specialization(guards = "!self.shouldTransitionOnPut()")
283-
static HashingStorage domStringKey(Node inliningTarget, DynamicObjectStorage self, TruffleString key, long keyHash, Object value,
284-
@Cached InlinedBranchProfile invalidateMroProfile,
282+
static HashingStorage domStringKey(DynamicObjectStorage self, TruffleString key, long keyHash, Object value,
285283
@Shared @CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
286-
self.setStringKey(key, value, dylib, inliningTarget, invalidateMroProfile);
284+
self.setStringKey(key, value, dylib);
287285
return self;
288286
}
289287

@@ -333,9 +331,8 @@ public abstract HashingStorage execute(Frame frame, Node inliningTarget, Dynamic
333331
static HashingStorage domStringKey(Node inliningTarget, DynamicObjectStorage self, Object key, @SuppressWarnings("unused") long keyHash, Object value,
334332
@SuppressWarnings("unused") boolean transition, DynamicObjectLibrary dylib,
335333
@SuppressWarnings("unused") @Cached PyUnicodeCheckExactNode isBuiltinString,
336-
@Cached CastBuiltinStringToTruffleStringNode castStr,
337-
@Cached InlinedBranchProfile invalidateMroProfile) {
338-
self.setStringKey(castStr.execute(inliningTarget, key), value, dylib, inliningTarget, invalidateMroProfile);
334+
@Cached CastBuiltinStringToTruffleStringNode castStr) {
335+
self.setStringKey(castStr.execute(inliningTarget, key), value, dylib);
339336
return self;
340337
}
341338

@@ -405,10 +402,9 @@ static HashingStorage empty(Frame frame, Node inliningTarget, @SuppressWarnings(
405402
}
406403

407404
@Specialization(guards = "!self.shouldTransitionOnPut()")
408-
static HashingStorage domStringKey(Node inliningTarget, DynamicObjectStorage self, TruffleString key, Object value,
409-
@Cached InlinedBranchProfile invalidateMroProfile,
405+
static HashingStorage domStringKey(DynamicObjectStorage self, TruffleString key, Object value,
410406
@Shared @CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
411-
self.setStringKey(key, value, dylib, inliningTarget, invalidateMroProfile);
407+
self.setStringKey(key, value, dylib);
412408
return self;
413409
}
414410

@@ -447,6 +443,10 @@ static HashingStorage foreign(Node inliningTarget, ForeignHashingStorage self, O
447443
return self;
448444
}
449445

446+
public static HashingStorageSetItem getUncached() {
447+
return HashingStorageSetItemNodeGen.getUncached();
448+
}
449+
450450
@GenerateUncached
451451
@GenerateInline
452452
@GenerateCached(false)
@@ -459,9 +459,8 @@ public abstract HashingStorage execute(Frame frame, Node inliningTarget, Dynamic
459459
static HashingStorage domStringKey(Node inliningTarget, DynamicObjectStorage self, Object key, Object value,
460460
@SuppressWarnings("unused") boolean transition, DynamicObjectLibrary dylib,
461461
@SuppressWarnings("unused") @Cached PyUnicodeCheckExactNode isBuiltinString,
462-
@Cached CastBuiltinStringToTruffleStringNode castStr,
463-
@Cached InlinedBranchProfile invalidateMroProfile) {
464-
self.setStringKey(castStr.execute(inliningTarget, key), value, dylib, inliningTarget, invalidateMroProfile);
462+
@Cached CastBuiltinStringToTruffleStringNode castStr) {
463+
self.setStringKey(castStr.execute(inliningTarget, key), value, dylib);
465464
return self;
466465
}
467466

@@ -542,7 +541,6 @@ static Object domStringKey(Frame frame, Node inliningTarget, DynamicObjectStorag
542541
@Cached PyUnicodeCheckExactNode isBuiltinString,
543542
@Cached CastBuiltinStringToTruffleStringNode castStr,
544543
@Exclusive @Cached PyObjectHashNode hashNode,
545-
@Exclusive @Cached InlinedBranchProfile invalidateMroProfile,
546544
@CachedLibrary(limit = "3") DynamicObjectLibrary dylib) {
547545
if (!isBuiltinString.execute(inliningTarget, keyObj)) {
548546
// Just for the potential side effects
@@ -557,16 +555,10 @@ static Object domStringKey(Frame frame, Node inliningTarget, DynamicObjectStorag
557555
return null;
558556
} else {
559557
dylib.put(store, key, PNone.NO_VALUE);
560-
self.invalidateAttributeInMROFinalAssumption(key, inliningTarget, invalidateMroProfile);
561558
return val;
562559
}
563560
} else {
564-
if (dylib.putIfPresent(store, key, PNone.NO_VALUE)) {
565-
self.invalidateAttributeInMROFinalAssumption(key, inliningTarget, invalidateMroProfile);
566-
return true;
567-
} else {
568-
return false;
569-
}
561+
return dylib.putIfPresent(store, key, PNone.NO_VALUE);
570562
}
571563
}
572564

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonBuiltinClass.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ public PythonBuiltinClassType getType() {
9999
public void onAttributeUpdate(TruffleString key, Object newValue) {
100100
assert !PythonContext.get(null).isCoreInitialized();
101101
// Ideally, startup code should not create ASTs that rely on assumptions of props of
102-
// builtins. So there should be no assumptions to invalidate yet
103-
assert !getMethodResolutionOrder().invalidateAttributeInMROFinalAssumptions(key);
102+
// builtins
103+
assert getMethodResolutionOrder().getFinalAttributeAssumption(key) == null;
104104
assert checkSpecialMethodUpdate(key, newValue);
105105
// NO_VALUE changes MRO lookup results without actually changing any Shapes in the MRO, this
106106
// can prevent some optimizations, so it is best to avoid any code that triggers such code

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,12 +282,6 @@ public boolean hasMroShapeSubTypes() {
282282
return mroShapeSubTypes != null;
283283
}
284284

285-
@Override
286-
public boolean canSkipOnAttributeUpdate(TruffleString key, @SuppressWarnings("unused") Object newValue, TruffleString.CodePointLengthNode codePointLengthNode,
287-
TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
288-
return super.canSkipOnAttributeUpdate(key, newValue, codePointLengthNode, codePointAtIndexNode) && mroShapeSubTypes == null;
289-
}
290-
291285
@TruffleBoundary
292286
@Override
293287
public void onAttributeUpdate(TruffleString key, Object newValue) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java‎

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,16 @@
3030

3131
import com.oracle.graal.python.PythonLanguage;
3232
import com.oracle.graal.python.builtins.objects.PNone;
33+
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
3334
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
3435
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
3536
import com.oracle.graal.python.builtins.objects.cext.capi.PythonClassNativeWrapper;
3637
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitionsFactory.PythonToNativeNodeGen;
38+
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
39+
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes;
3740
import com.oracle.graal.python.builtins.objects.dict.PDict;
3841
import com.oracle.graal.python.builtins.objects.object.PythonObject;
42+
import com.oracle.graal.python.builtins.objects.referencetype.PReferenceType;
3943
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
4044
import com.oracle.graal.python.builtins.objects.type.TypeNodes.ComputeMroNode;
4145
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetSubclassesAsArrayNode;
@@ -47,6 +51,7 @@
4751
import com.oracle.graal.python.runtime.object.PFactory;
4852
import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage;
4953
import com.oracle.truffle.api.CompilerAsserts;
54+
import com.oracle.truffle.api.CompilerDirectives;
5055
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
5156
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5257
import com.oracle.truffle.api.nodes.Node;
@@ -160,6 +165,9 @@ Object getBase() {
160165
}
161166

162167
public void setMRO(PythonAbstractClass[] mro) {
168+
if (methodResolutionOrder != null) {
169+
methodResolutionOrder.lookupChanged();
170+
}
163171
methodResolutionOrder = new MroSequenceStorage(name, mro);
164172
}
165173

@@ -207,18 +215,10 @@ public void setAttribute(TruffleString key, Object value) {
207215
onAttributeUpdate(key, value);
208216
}
209217

210-
/**
211-
* Fast-path check designed for PE code.
212-
*/
213-
public boolean canSkipOnAttributeUpdate(TruffleString key, @SuppressWarnings("unused") Object value, TruffleString.CodePointLengthNode codePointLengthNode,
214-
TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
215-
return !methodResolutionOrder.hasAttributeInMROFinalAssumptions() &&
216-
!TpSlots.canBeSpecialMethod(key, codePointLengthNode, codePointAtIndexNode);
217-
}
218-
219218
@TruffleBoundary
220219
public void onAttributeUpdate(TruffleString key, Object value) {
221-
methodResolutionOrder.invalidateAttributeInMROFinalAssumptions(key);
220+
callOnAttributeUpdateOnSubclasses(subClasses, key, value);
221+
methodResolutionOrder.invalidateFinalAttributeAssumption(key);
222222
if (TpSlots.canBeSpecialMethod(key, CodePointLengthNode.getUncached(), CodePointAtIndexNode.getUncached())) {
223223
if (this.tpSlots != null) {
224224
// This is called during type instantiation from copyDictSlots when the tp slots are
@@ -228,6 +228,36 @@ public void onAttributeUpdate(TruffleString key, Object value) {
228228
}
229229
}
230230

231+
@TruffleBoundary
232+
public static void onAttributeUpdateNative(PythonAbstractNativeObject nativeClass, TruffleString key, Object value) {
233+
assert TypeNodes.IsTypeNode.executeUncached(nativeClass);
234+
callOnAttributeUpdateOnSubclasses(GetSubclassesNode.executeUncached(nativeClass), key, value);
235+
TypeNodes.GetMroStorageNode.executeUncached(nativeClass).invalidateFinalAttributeAssumption(key);
236+
if (TpSlots.canBeSpecialMethod(key, CodePointLengthNode.getUncached(), CodePointAtIndexNode.getUncached())) {
237+
TpSlots.updateSlot(nativeClass, key);
238+
}
239+
}
240+
241+
private static void callOnAttributeUpdateOnSubclasses(PDict subClasses, TruffleString key, Object value) {
242+
if (subClasses != null) {
243+
HashingStorage dictStorage = subClasses.getDictStorage();
244+
HashingStorageNodes.HashingStorageIterator it = HashingStorageNodes.HashingStorageGetIterator.executeUncached(dictStorage);
245+
while (HashingStorageNodes.HashingStorageIteratorNext.executeUncached(dictStorage, it)) {
246+
PReferenceType ref = (PReferenceType) HashingStorageNodes.HashingStorageIteratorValue.executeUncached(dictStorage, it);
247+
Object subclass = ref.getObject();
248+
if (subclass != null) {
249+
if (subclass instanceof PythonManagedClass managedClass) {
250+
managedClass.onAttributeUpdate(key, value);
251+
} else if (subclass instanceof PythonAbstractNativeObject nativeClass) {
252+
onAttributeUpdateNative(nativeClass, key, value);
253+
} else {
254+
throw CompilerDirectives.shouldNotReachHere("Unexpected subclass type");
255+
}
256+
}
257+
}
258+
}
259+
}
260+
231261
/**
232262
* This method supports initialization and solves boot-order problems and should not normally be
233263
* used.
@@ -276,12 +306,10 @@ public final void setBases(Node node, Object newBaseClass, PythonAbstractClass[]
276306
// for what follows see also typeobject.c#type_set_bases()
277307
this.base = newBaseClass;
278308
this.baseClasses = newBaseClasses;
279-
this.methodResolutionOrder.lookupChanged();
280309
this.setMRO(ComputeMroNode.doSlowPath(node, this));
281310

282311
for (PythonAbstractClass scls : subclassesArray) {
283312
if (scls instanceof PythonManagedClass pmc) {
284-
pmc.methodResolutionOrder.lookupChanged();
285313
pmc.setMRO(ComputeMroNode.doSlowPath(node, scls));
286314
}
287315
}
@@ -294,14 +322,12 @@ public final void setBases(Node node, Object newBaseClass, PythonAbstractClass[]
294322
this.base = oldBase;
295323
this.baseClasses = oldBaseClasses;
296324
}
297-
this.methodResolutionOrder.lookupChanged();
298325
this.setMRO(oldMRO);
299326

300327
for (int i = 0; i < subclassesArray.length; i++) {
301328
PythonAbstractClass scls = subclassesArray[i];
302329
if (oldSubClasssMROs[i] != null) {
303330
PythonManagedClass pmc = (PythonManagedClass) scls;
304-
pmc.methodResolutionOrder.lookupChanged();
305331
pmc.setMRO(oldSubClasssMROs[i]);
306332
}
307333
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeBuiltins.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ static Object doManaged(PythonManagedClass self,
785785
@Shared @Cached GetDictIfExistsNode getDict) {
786786
PDict dict = getDict.execute(self);
787787
if (dict == null) {
788-
dict = PFactory.createDictFixedStorage(language, self, self.getMethodResolutionOrder());
788+
dict = PFactory.createDictFixedStorage(language, self);
789789
// The mapping is unmodifiable, so we don't have to assign it back
790790
}
791791
return PFactory.createMappingproxy(language, dict);

0 commit comments

Comments
 (0)