Skip to content

Commit 7ba69f9

Browse files
lukasstadlerlukasstadler
committed
further simplifications in MRO lookup optimization
1 parent d37e320 commit 7ba69f9

File tree

5 files changed

+94
-71
lines changed

5 files changed

+94
-71
lines changed

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

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,16 @@ public class PythonClass extends PythonObject {
6363

6464
@CompilationFinal(dimensions = 1) private PythonClass[] baseClasses;
6565
@CompilationFinal(dimensions = 1) private PythonClass[] methodResolutionOrder;
66-
private CyclicAssumption lookupStableAssumption;
67-
private Map<Object, List<Assumption>> attributesInMROFinalAssumptions;
66+
67+
/**
68+
* This assumption will be invalidated whenever the mro changes.
69+
*/
70+
private final CyclicAssumption lookupStableAssumption;
71+
/**
72+
* These assumptions will be invalidated whenever the value of the given slot changes. All
73+
* assumptions will be invalidated if the mro changes.
74+
*/
75+
private final Map<String, List<Assumption>> attributesInMROFinalAssumptions = new HashMap<>();
6876

6977
private final Set<PythonClass> subClasses = Collections.newSetFromMap(new WeakHashMap<PythonClass, Boolean>());
7078
private final Shape instanceShape;
@@ -79,8 +87,6 @@ public final boolean isBuiltin() {
7987
public PythonClass(PythonClass typeClass, String name, PythonClass... baseClasses) {
8088
super(typeClass, freshShape() /* do not inherit layout from the TypeClass */);
8189
this.className = name;
82-
this.lookupStableAssumption = new CyclicAssumption(className);
83-
this.attributesInMROFinalAssumptions = new HashMap<>();
8490

8591
assert baseClasses.length > 0;
8692
if (baseClasses.length == 1 && baseClasses[0] == null) {
@@ -90,6 +96,7 @@ public PythonClass(PythonClass typeClass, String name, PythonClass... baseClasse
9096
}
9197

9298
this.flags = new FlagsContainer(getSuperClass());
99+
this.lookupStableAssumption = new CyclicAssumption(className);
93100

94101
// Compute MRO
95102
computeMethodResolutionOrder();
@@ -113,8 +120,8 @@ public Assumption getLookupStableAssumption() {
113120
return lookupStableAssumption.getAssumption();
114121
}
115122

116-
@TruffleBoundary
117-
public Assumption createAttributeInMROFinalAssumption(Object name) {
123+
public Assumption createAttributeInMROFinalAssumption(String name) {
124+
CompilerAsserts.neverPartOfCompilation();
118125
List<Assumption> attrAssumptions = attributesInMROFinalAssumptions.getOrDefault(name, null);
119126
if (attrAssumptions == null) {
120127
attrAssumptions = new ArrayList<>();
@@ -126,8 +133,8 @@ public Assumption createAttributeInMROFinalAssumption(Object name) {
126133
return assumption;
127134
}
128135

129-
@TruffleBoundary
130-
public void addAttributeInMROFinalAssumption(Object name, Assumption assumption) {
136+
public void addAttributeInMROFinalAssumption(String name, Assumption assumption) {
137+
CompilerAsserts.neverPartOfCompilation();
131138
List<Assumption> attrAssumptions = attributesInMROFinalAssumptions.getOrDefault(name, null);
132139
if (attrAssumptions == null) {
133140
attrAssumptions = new ArrayList<>();
@@ -137,16 +144,27 @@ public void addAttributeInMROFinalAssumption(Object name, Assumption assumption)
137144
attrAssumptions.add(assumption);
138145
}
139146

140-
@TruffleBoundary
141-
public void invalidateAttributeInMROFinalAssumptions(Object name) {
147+
public void invalidateAttributeInMROFinalAssumptions(String name) {
148+
CompilerAsserts.neverPartOfCompilation();
142149
List<Assumption> assumptions = attributesInMROFinalAssumptions.getOrDefault(name, new ArrayList<>());
143-
for (Assumption assumption : assumptions) {
144-
assumption.invalidate(className + "." + name);
150+
if (!assumptions.isEmpty()) {
151+
String message = className + "." + name;
152+
for (Assumption assumption : assumptions) {
153+
assumption.invalidate(message);
154+
}
145155
}
146156
}
147157

148-
@TruffleBoundary
158+
/**
159+
* This method needs to be called if the mro changes. (currently not used)
160+
*/
149161
public void lookupChanged() {
162+
CompilerAsserts.neverPartOfCompilation();
163+
for (List<Assumption> list : attributesInMROFinalAssumptions.values()) {
164+
for (Assumption assumption : list) {
165+
assumption.invalidate();
166+
}
167+
}
150168
lookupStableAssumption.invalidate();
151169
for (PythonClass subclass : getSubClasses()) {
152170
if (subclass != null) {
@@ -271,9 +289,18 @@ public void addMethod(PFunction method) {
271289

272290
@Override
273291
@TruffleBoundary
274-
public void setAttribute(Object name, Object value) {
275-
super.setAttribute(name, value);
276-
lookupChanged();
292+
public void setAttribute(Object key, Object value) {
293+
if (key instanceof String) {
294+
invalidateAttributeInMROFinalAssumptions((String) key);
295+
}
296+
super.setAttribute(key, value);
297+
}
298+
299+
@Override
300+
@TruffleBoundary
301+
public void deleteAttribute(String key) {
302+
invalidateAttributeInMROFinalAssumptions(key);
303+
super.deleteAttribute(key);
277304
}
278305

279306
@Override
@@ -292,6 +319,9 @@ public Object getAttribute(String name) {
292319
* used.
293320
*/
294321
public void unsafeSetSuperClass(PythonClass... newBaseClasses) {
322+
// TODO: if this is used outside bootstrapping, it needs to call
323+
// computeMethodResolutionOrder for subclasses.
324+
295325
assert getBaseClasses() == null || getBaseClasses().length == 0;
296326
this.baseClasses = newBaseClasses;
297327

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,6 @@ public static DeleteAttributeNode create(PNode object, PNode key) {
6363
public abstract Object execute(Object object, Object key);
6464

6565
@Specialization
66-
protected Object doClass(PythonClass cls, Object key,
67-
@Cached("createIdentityProfile()") ValueProfile setAttributeProfile,
68-
@Cached("create(__DELATTR__)") LookupAttributeInMRONode delAttributeLookup,
69-
@Cached("create()") CallBinaryMethodNode callDelAttribute) {
70-
cls.lookupChanged();
71-
cls.invalidateAttributeInMROFinalAssumptions(key);
72-
Object descr = setAttributeProfile.profile(delAttributeLookup.execute(cls));
73-
return callDelAttribute.executeObject(descr, cls, key);
74-
}
75-
76-
@Specialization(guards = "!isClass(object)")
7766
protected Object doIt(Object object, Object key,
7867
@Cached("createIdentityProfile()") ValueProfile setAttributeProfile,
7968
@Cached("create()") GetClassNode getClassNode,

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -103,45 +103,40 @@ public static LookupAttributeInMRONode create(String key) {
103103
public abstract Object execute(PythonClass klass);
104104

105105
final static class PythonClassAssumptionPair {
106-
public final PythonClass cls;
107106
public final Assumption assumption;
107+
public final Object value;
108108

109-
PythonClassAssumptionPair(PythonClass cls, Assumption assumption) {
110-
this.cls = cls;
109+
PythonClassAssumptionPair(Assumption assumption, Object value) {
111110
this.assumption = assumption;
111+
this.value = value;
112112
}
113113
}
114114

115115
protected PythonClassAssumptionPair findAttrClassAndAssumptionInMRO(PythonClass klass) {
116116
PythonClass[] mro = klass.getMethodResolutionOrder();
117-
Assumption attrAssumption = null;
117+
Assumption attrAssumption = klass.createAttributeInMROFinalAssumption(key);
118118
for (int i = 0; i < mro.length; i++) {
119119
PythonClass cls = mro[i];
120120
if (i > 0) {
121121
assert cls != klass : "MRO chain is incorrect: '" + klass + "' was found at position " + i;
122-
if (attrAssumption == null) {
123-
attrAssumption = cls.createAttributeInMROFinalAssumption(key);
124-
} else {
125-
cls.addAttributeInMROFinalAssumption(key, attrAssumption);
126-
}
122+
cls.addAttributeInMROFinalAssumption(key, attrAssumption);
127123
}
128124

129125
if (cls.getStorage().containsKey(key)) {
130-
return new PythonClassAssumptionPair(cls, attrAssumption);
126+
Object value = cls.getStorage().get(key);
127+
if (value != PNone.NO_VALUE) {
128+
return new PythonClassAssumptionPair(attrAssumption, value);
129+
}
131130
}
132131
}
133-
return new PythonClassAssumptionPair(null, attrAssumption);
132+
return new PythonClassAssumptionPair(attrAssumption, PNone.NO_VALUE);
134133
}
135134

136-
@Specialization(guards = {"klass == cachedKlass", "cachedClassInMROInfo.cls != null"}, limit = "5", assumptions = {"lookupStable",
137-
"cachedClassInMROInfo.assumption"})
135+
@Specialization(guards = {"klass == cachedKlass"}, limit = "5", assumptions = {"cachedClassInMROInfo.assumption"})
138136
protected Object lookupConstantMROCached(@SuppressWarnings("unused") PythonClass klass,
139137
@Cached("klass") @SuppressWarnings("unused") PythonClass cachedKlass,
140-
@Cached("cachedKlass.getLookupStableAssumption()") @SuppressWarnings("unused") Assumption lookupStable,
141-
@Cached("create()") @SuppressWarnings("unused") ReadAttributeFromObjectNode readAttributeNode,
142-
@Cached("findAttrClassAndAssumptionInMRO(cachedKlass)") @SuppressWarnings("unused") PythonClassAssumptionPair cachedClassInMROInfo,
143-
@Cached("readAttributeNode.execute(cachedClassInMROInfo.cls, key)") Object value) {
144-
return value;
138+
@Cached("findAttrClassAndAssumptionInMRO(cachedKlass)") PythonClassAssumptionPair cachedClassInMROInfo) {
139+
return cachedClassInMROInfo.value;
145140
}
146141

147142
protected ReadAttributeFromObjectNode[] create(int size) {

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
import com.oracle.truffle.api.dsl.NodeChildren;
5252
import com.oracle.truffle.api.dsl.Specialization;
5353
import com.oracle.truffle.api.frame.VirtualFrame;
54-
import com.oracle.truffle.api.profiles.ConditionProfile;
5554
import com.oracle.truffle.api.profiles.ValueProfile;
5655

5756
@NodeChildren({@NodeChild(value = "object", type = PNode.class), @NodeChild(value = "key", type = PNode.class), @NodeChild(value = "rhs", type = PNode.class)})
@@ -90,20 +89,6 @@ public PNode getPrimaryNode() {
9089
}
9190

9291
@Specialization
93-
protected Object doClass(PythonClass cls, Object key, Object value,
94-
@Cached("createIdentityProfile()") ValueProfile setAttributeProfile,
95-
@Cached("create(__SETATTR__)") LookupAttributeInMRONode setAttributeLookup,
96-
@Cached("create()") CallTernaryMethodNode callSetAttribute,
97-
@Cached("createBinaryProfile()") ConditionProfile isAddingAttributeProfile) {
98-
if (isAddingAttributeProfile.profile(!cls.getStorage().containsKey(key))) {
99-
cls.lookupChanged();
100-
}
101-
cls.invalidateAttributeInMROFinalAssumptions(key);
102-
Object descr = setAttributeProfile.profile(setAttributeLookup.execute(cls));
103-
return callSetAttribute.execute(descr, cls, key, value);
104-
}
105-
106-
@Specialization(guards = "!isClass(object)")
10792
protected Object doIt(Object object, Object key, Object value,
10893
@Cached("createIdentityProfile()") ValueProfile setAttributeProfile,
10994
@Cached("create()") GetClassNode getClassNode,

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

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,31 @@
4242

4343
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4444
import com.oracle.graal.python.builtins.objects.str.PString;
45-
import com.oracle.graal.python.nodes.PNode;
45+
import com.oracle.graal.python.builtins.objects.type.PythonClass;
46+
import com.oracle.graal.python.nodes.PBaseNode;
47+
import com.oracle.graal.python.runtime.PythonOptions;
4648
import com.oracle.truffle.api.Assumption;
4749
import com.oracle.truffle.api.CompilerDirectives;
4850
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4951
import com.oracle.truffle.api.dsl.Cached;
50-
import com.oracle.truffle.api.dsl.NodeChild;
51-
import com.oracle.truffle.api.dsl.NodeChildren;
52+
import com.oracle.truffle.api.dsl.ImportStatic;
5253
import com.oracle.truffle.api.dsl.Specialization;
5354
import com.oracle.truffle.api.object.FinalLocationException;
5455
import com.oracle.truffle.api.object.IncompatibleLocationException;
5556
import com.oracle.truffle.api.object.Location;
5657
import com.oracle.truffle.api.object.Property;
5758
import com.oracle.truffle.api.object.Shape;
59+
import com.oracle.truffle.api.profiles.ConditionProfile;
60+
61+
@ImportStatic(PythonOptions.class)
62+
public abstract class WriteAttributeToObjectNode extends PBaseNode {
5863

59-
@NodeChildren({@NodeChild(value = "object", type = PNode.class), @NodeChild(value = "key", type = PNode.class), @NodeChild(value = "value", type = PNode.class)})
60-
public abstract class WriteAttributeToObjectNode extends PNode {
6164
public abstract boolean execute(Object primary, Object key, Object value);
6265

66+
public abstract boolean execute(Object primary, String key, Object value);
67+
6368
public static WriteAttributeToObjectNode create() {
64-
return WriteAttributeToObjectNodeGen.create(null, null, null);
69+
return WriteAttributeToObjectNodeGen.create();
6570
}
6671

6772
protected Location getLocationOrNull(Property prop) {
@@ -76,24 +81,38 @@ protected Object attrKey(Object key) {
7681
}
7782
}
7883

84+
private final ConditionProfile isClassProfile = ConditionProfile.createBinaryProfile();
85+
86+
private void handlePythonClass(PythonObject object, Object key) {
87+
if (isClassProfile.profile(object instanceof PythonClass)) {
88+
if (key instanceof String) {
89+
((PythonClass) object).invalidateAttributeInMROFinalAssumptions((String) key);
90+
}
91+
}
92+
}
93+
7994
@SuppressWarnings("unused")
8095
@Specialization(guards = {
8196
"object.getStorage().getShape() == cachedShape",
8297
"!layoutAssumption.isValid()"
8398
})
84-
protected Object updateShapeAndWrite(PythonObject object, Object key, Object value,
99+
protected boolean updateShapeAndWrite(PythonObject object, Object key, Object value,
85100
@Cached("object.getStorage().getShape()") Shape cachedShape,
86101
@Cached("cachedShape.getValidAssumption()") Assumption layoutAssumption,
87102
@Cached("create()") WriteAttributeToObjectNode nextNode) {
88103
object.getStorage().updateShape();
89104
return nextNode.execute(object, key, value);
90105
}
91106

107+
protected static boolean compareKey(Object cachedKey, Object key) {
108+
return cachedKey == key;
109+
}
110+
92111
@SuppressWarnings("unused")
93112
@Specialization(limit = "getIntOption(getContext(), AttributeAccessInlineCacheMaxDepth)", //
94113
guards = {
95114
"object.getStorage().getShape() == cachedShape",
96-
"cachedKey == key",
115+
"compareKey(cachedKey, key)",
97116
"loc != null",
98117
"loc.canSet(value)"
99118
}, //
@@ -108,6 +127,7 @@ protected boolean doDirect(PythonObject object, Object key, Object value,
108127
@Cached("cachedShape.getValidAssumption()") Assumption layoutAssumption,
109128
@Cached("getLocationOrNull(cachedShape.getProperty(attrKey))") Location loc) {
110129
try {
130+
handlePythonClass(object, attrKey);
111131
loc.set(object.getStorage(), value);
112132
} catch (IncompatibleLocationException | FinalLocationException e) {
113133
CompilerDirectives.transferToInterpreter();
@@ -121,7 +141,7 @@ protected boolean doDirect(PythonObject object, Object key, Object value,
121141
@Specialization(limit = "getIntOption(getContext(), AttributeAccessInlineCacheMaxDepth)", //
122142
guards = {
123143
"object.getStorage().getShape() == cachedShape",
124-
"cachedKey == key",
144+
"compareKey(cachedKey, key)",
125145
"loc == null || !loc.canSet(value)",
126146
"newLoc.canSet(value)"
127147
}, //
@@ -139,6 +159,7 @@ protected boolean defineDirect(PythonObject object, Object key, Object value,
139159
@Cached("newShape.getValidAssumption()") Assumption newLayoutAssumption,
140160
@Cached("getLocationOrNull(newShape.getProperty(attrKey))") Location newLoc) {
141161
try {
162+
handlePythonClass(object, attrKey);
142163
newLoc.set(object.getStorage(), value, cachedShape, newShape);
143164
} catch (IncompatibleLocationException e) {
144165
CompilerDirectives.transferToInterpreter();
@@ -151,13 +172,16 @@ protected boolean defineDirect(PythonObject object, Object key, Object value,
151172
@TruffleBoundary
152173
@Specialization(replaces = {"doDirect", "defineDirect"}, guards = {"object.getStorage().getShape().isValid()"})
153174
protected boolean doIndirect(PythonObject object, Object key, Object value) {
154-
object.setAttribute(attrKey(key), value);
175+
Object attrKey = attrKey(key);
176+
handlePythonClass(object, attrKey);
177+
object.setAttribute(attrKey, value);
155178
return true;
156179
}
157180

158-
@TruffleBoundary
159181
@Specialization(guards = "!object.getStorage().getShape().isValid()")
160182
protected boolean defineDirect(PythonObject object, Object key, Object value) {
183+
CompilerDirectives.transferToInterpreter();
184+
161185
object.getStorage().updateShape();
162186
return doIndirect(object, key, value);
163187
}

0 commit comments

Comments
 (0)