Skip to content

Commit 7f57e2f

Browse files
timfeltimfel
committed
make sure we check assumptions when reading quasi-immutable fields off PFunction
1 parent 16d0499 commit 7f57e2f

File tree

3 files changed

+60
-31
lines changed

3 files changed

+60
-31
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/PFunction.java

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,32 @@
3535
import com.oracle.graal.python.builtins.objects.type.LazyPythonClass;
3636
import com.oracle.graal.python.nodes.SpecialMethodNames;
3737
import com.oracle.graal.python.nodes.generator.GeneratorFunctionRootNode;
38+
import com.oracle.truffle.api.Assumption;
3839
import com.oracle.truffle.api.CompilerAsserts;
40+
import com.oracle.truffle.api.CompilerDirectives;
3941
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
4042
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4143
import com.oracle.truffle.api.RootCallTarget;
44+
import com.oracle.truffle.api.Truffle;
4245
import com.oracle.truffle.api.nodes.RootNode;
4346
import com.oracle.truffle.api.object.DynamicObject;
4447
import com.oracle.truffle.api.source.SourceSection;
45-
import com.oracle.truffle.api.utilities.CyclicAssumption;
4648

4749
public class PFunction extends PythonObject {
4850
private static final Object[] EMPTY_DEFAULTS = new Object[0];
4951
private final String name;
5052
private final String enclosingClassName;
51-
private final CyclicAssumption codeStableAssumption = new CyclicAssumption("function code unchanged");
52-
private final CyclicAssumption defaultsStableAssumption = new CyclicAssumption("function defaults unchanged");
53+
private final Assumption codeStableAssumption = Truffle.getRuntime().createAssumption("function code unchanged for " + toString());
54+
private final Assumption defaultsStableAssumption = Truffle.getRuntime().createAssumption("function defaults unchanged " + toString());
5355
private final PythonObject globals;
5456
private final PCell[] closure;
5557
private final boolean isStatic;
5658
@CompilationFinal private PCode code;
59+
private PCode uncachedCode;
5760
@CompilationFinal(dimensions = 1) private Object[] defaultValues;
61+
private Object[] uncachedDefaultValues;
5862
@CompilationFinal(dimensions = 1) private PKeyword[] kwDefaultValues;
63+
private PKeyword[] uncachedKwDefaultValues;
5964

6065
public PFunction(LazyPythonClass clazz, String name, String enclosingClassName, RootCallTarget callTarget, PythonObject globals, PCell[] closure) {
6166
this(clazz, name, enclosingClassName, callTarget, globals, EMPTY_DEFAULTS, PKeyword.EMPTY_KEYWORDS, closure);
@@ -65,12 +70,12 @@ public PFunction(LazyPythonClass clazz, String name, String enclosingClassName,
6570
PCell[] closure) {
6671
super(clazz);
6772
this.name = name;
68-
this.code = new PCode(PythonBuiltinClassType.PCode, callTarget);
73+
this.code = this.uncachedCode = new PCode(PythonBuiltinClassType.PCode, callTarget);
6974
this.isStatic = name.equals(SpecialMethodNames.__NEW__);
7075
this.enclosingClassName = enclosingClassName;
7176
this.globals = globals;
72-
this.defaultValues = defaultValues == null ? EMPTY_DEFAULTS : defaultValues;
73-
this.kwDefaultValues = kwDefaultValues == null ? PKeyword.EMPTY_KEYWORDS : kwDefaultValues;
77+
this.defaultValues = this.uncachedDefaultValues = defaultValues == null ? EMPTY_DEFAULTS : defaultValues;
78+
this.kwDefaultValues = this.uncachedKwDefaultValues = kwDefaultValues == null ? PKeyword.EMPTY_KEYWORDS : kwDefaultValues;
7479
this.closure = closure;
7580
addDefaultConstants(this.getStorage(), name, enclosingClassName);
7681
}
@@ -86,14 +91,14 @@ public boolean isStatic() {
8691
}
8792

8893
public RootCallTarget getCallTarget() {
89-
return code.getRootCallTarget();
94+
return getCode().getRootCallTarget();
9095
}
9196

92-
public CyclicAssumption getCodeStableAssumption() {
97+
public Assumption getCodeStableAssumption() {
9398
return codeStableAssumption;
9499
}
95100

96-
public CyclicAssumption getDefaultsStableAssumption() {
101+
public Assumption getDefaultsStableAssumption() {
97102
return defaultsStableAssumption;
98103
}
99104

@@ -110,7 +115,7 @@ public String getName() {
110115
}
111116

112117
public Arity getArity() {
113-
return code.getArity();
118+
return getCode().getArity();
114119
}
115120

116121
public PCell[] getClosure() {
@@ -136,31 +141,52 @@ public final String toString() {
136141
}
137142

138143
public PCode getCode() {
139-
return code;
144+
Assumption assumption = this.codeStableAssumption;
145+
if (CompilerDirectives.isCompilationConstant(this) && CompilerDirectives.isCompilationConstant(assumption)) {
146+
if (assumption.isValid()) {
147+
return code;
148+
}
149+
}
150+
return uncachedCode;
140151
}
141152

142153
public void setCode(PCode code) {
143-
this.code = code;
154+
codeStableAssumption.invalidate("code changed for function " + getName());
155+
this.code = this.uncachedCode = code;
144156
}
145157

146158
public String getEnclosingClassName() {
147159
return enclosingClassName;
148160
}
149161

150162
public Object[] getDefaults() {
151-
return defaultValues;
163+
Assumption assumption = this.defaultsStableAssumption;
164+
if (CompilerDirectives.isCompilationConstant(this) && CompilerDirectives.isCompilationConstant(assumption)) {
165+
if (assumption.isValid()) {
166+
return defaultValues;
167+
}
168+
}
169+
return uncachedDefaultValues;
152170
}
153171

154172
public void setDefaults(Object[] defaults) {
155-
this.defaultValues = defaults;
173+
this.defaultsStableAssumption.invalidate("defaults changed for function " + getName());
174+
this.defaultValues = this.uncachedDefaultValues = defaults;
156175
}
157176

158177
public PKeyword[] getKwDefaults() {
159-
return kwDefaultValues;
178+
Assumption assumption = this.defaultsStableAssumption;
179+
if (CompilerDirectives.isCompilationConstant(this) && CompilerDirectives.isCompilationConstant(assumption)) {
180+
if (assumption.isValid()) {
181+
return kwDefaultValues;
182+
}
183+
}
184+
return uncachedKwDefaultValues;
160185
}
161186

162187
public void setKwDefaults(PKeyword[] defaults) {
163-
this.kwDefaultValues = defaults;
188+
this.defaultsStableAssumption.invalidate("kw defaults changed for function " + getName());
189+
this.kwDefaultValues = this.uncachedKwDefaultValues = defaults;
164190
}
165191

166192
@TruffleBoundary

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/CallDispatchNode.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
package com.oracle.graal.python.nodes.call;
2727

2828
import com.oracle.graal.python.PythonLanguage;
29+
import com.oracle.graal.python.builtins.objects.code.PCode;
2930
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
3031
import com.oracle.graal.python.builtins.objects.function.PFunction;
3132
import com.oracle.graal.python.runtime.PythonOptions;
@@ -74,27 +75,39 @@ protected Assumption singleContextAssumption() {
7475

7576
public abstract Object executeCall(VirtualFrame frame, PBuiltinFunction callee, Object[] arguments);
7677

77-
@Specialization(guards = {"callee == cachedCallee"}, limit = "getCallSiteInlineCacheMaxDepth()", assumptions = "singleContextAssumption()")
78+
// We only have a single context and this function never changed its code
79+
@Specialization(guards = {"callee == cachedCallee"}, limit = "getCallSiteInlineCacheMaxDepth()", assumptions = {"singleContextAssumption()", "cachedCallee.getCodeStableAssumption()"})
7880
protected Object callFunctionCached(VirtualFrame frame, @SuppressWarnings("unused") PFunction callee, Object[] arguments,
7981
@SuppressWarnings("unused") @Cached("callee") PFunction cachedCallee,
8082
@Cached("createInvokeNode(cachedCallee)") InvokeNode invoke) {
8183
return invoke.execute(frame, arguments);
8284
}
8385

84-
@Specialization(guards = {"callee == cachedCallee"}, limit = "getCallSiteInlineCacheMaxDepth()", assumptions = "singleContextAssumption()")
85-
protected Object callBuiltinFunctionCached(VirtualFrame frame, @SuppressWarnings("unused") PBuiltinFunction callee, Object[] arguments,
86-
@SuppressWarnings("unused") @Cached("callee") PBuiltinFunction cachedCallee,
86+
// We only have a single context and this function changed its code before, but now it's
87+
// constant
88+
@Specialization(guards = {"callee == cachedCallee", "callee.getCode() == cachedCode"}, limit = "getCallSiteInlineCacheMaxDepth()", assumptions = {"singleContextAssumption()"})
89+
protected Object callFunctionCachedCode(VirtualFrame frame, @SuppressWarnings("unused") PFunction callee, Object[] arguments,
90+
@SuppressWarnings("unused") @Cached("callee") PFunction cachedCallee,
91+
@SuppressWarnings("unused") @Cached("callee.getCode()") PCode cachedCode,
8792
@Cached("createInvokeNode(cachedCallee)") InvokeNode invoke) {
8893
return invoke.execute(frame, arguments);
8994
}
9095

91-
@Specialization(guards = "callee.getCallTarget() == ct", limit = "getCallSiteInlineCacheMaxDepth()")
96+
// We have multiple contexts, don't cache the objects so that contexts can be cleaned up
97+
@Specialization(guards = {"callee.getCallTarget() == ct"}, limit = "getCallSiteInlineCacheMaxDepth()")
9298
protected Object callFunctionCachedCt(VirtualFrame frame, PFunction callee, Object[] arguments,
9399
@SuppressWarnings("unused") @Cached("callee.getCallTarget()") RootCallTarget ct,
94100
@Cached("createCtInvokeNode(callee)") CallTargetInvokeNode invoke) {
95101
return invoke.execute(frame, callee.getGlobals(), callee.getClosure(), arguments);
96102
}
97103

104+
@Specialization(guards = {"callee == cachedCallee"}, limit = "getCallSiteInlineCacheMaxDepth()", assumptions = "singleContextAssumption()")
105+
protected Object callBuiltinFunctionCached(VirtualFrame frame, @SuppressWarnings("unused") PBuiltinFunction callee, Object[] arguments,
106+
@SuppressWarnings("unused") @Cached("callee") PBuiltinFunction cachedCallee,
107+
@Cached("createInvokeNode(cachedCallee)") InvokeNode invoke) {
108+
return invoke.execute(frame, arguments);
109+
}
110+
98111
@Specialization(guards = "callee.getCallTarget() == ct", limit = "getCallSiteInlineCacheMaxDepth()")
99112
protected Object callBuiltinFunctionCachedCt(VirtualFrame frame, @SuppressWarnings("unused") PBuiltinFunction callee, Object[] arguments,
100113
@SuppressWarnings("unused") @Cached("callee.getCallTarget()") RootCallTarget ct,

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/InvokeNode.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,16 +92,6 @@ protected final void optionallySetClassBodySpecial(Object[] arguments, CallTarge
9292
}
9393
}
9494

95-
@TruffleBoundary
96-
protected static Arity getArity(PFunction callee) {
97-
return callee.getArity();
98-
}
99-
100-
@TruffleBoundary
101-
protected static Arity getArity(PBuiltinFunction callee) {
102-
return callee.getArity();
103-
}
104-
10595
protected static boolean isBuiltin(Object callee) {
10696
return callee instanceof PBuiltinFunction || callee instanceof PBuiltinMethod;
10797
}

0 commit comments

Comments
 (0)