Skip to content

Commit 0ddcc33

Browse files
chumerchumer
committed
[GR-19736] Fix invalid context property caching.
PullRequest: graalpython/1013
2 parents ba5e61f + 0725ffd commit 0ddcc33

File tree

4 files changed

+137
-31
lines changed

4 files changed

+137
-31
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727

2828
import java.io.IOException;
2929
import java.util.ArrayList;
30+
import java.util.Arrays;
3031
import java.util.concurrent.ConcurrentHashMap;
3132
import java.util.concurrent.Semaphore;
3233
import com.oracle.graal.python.util.Supplier;
3334
import java.util.logging.Level;
3435

3536
import org.graalvm.options.OptionDescriptors;
37+
import org.graalvm.options.OptionKey;
3638
import org.graalvm.options.OptionValues;
3739

3840
import com.oracle.graal.python.builtins.Python3Core;
@@ -144,6 +146,9 @@ public final class PythonLanguage extends TruffleLanguage<PythonContext> {
144146
*/
145147
private Boolean isWithThread = null;
146148

149+
@CompilationFinal(dimensions = 1) private volatile Object[] engineOptionsStorage;
150+
@CompilationFinal private volatile OptionValues engineOptions;
151+
147152
public static int getNumberOfSpecialSingletons() {
148153
return CONTEXT_INSENSITIVE_SINGLETONS.length;
149154
}
@@ -197,9 +202,32 @@ protected PythonContext createContext(Env env) {
197202
Python3Core newCore = new Python3Core(new PythonParserImpl(env));
198203
final PythonContext context = new PythonContext(this, env, newCore);
199204
context.initializeHomeAndPrefixPaths(env, getLanguageHome());
205+
206+
Object[] engineOptionsUnroll = this.engineOptionsStorage;
207+
if (engineOptionsUnroll == null) {
208+
this.engineOptionsStorage = engineOptionsUnroll = PythonOptions.createEngineOptionValuesStorage(env);
209+
} else {
210+
assert Arrays.equals(engineOptionsUnroll, PythonOptions.createEngineOptionValuesStorage(env)) : "invalid engine options";
211+
}
212+
213+
OptionValues options = this.engineOptions;
214+
if (options == null) {
215+
this.engineOptions = options = PythonOptions.createEngineOptions(env);
216+
} else {
217+
assert options.equals(PythonOptions.createEngineOptions(env)) : "invalid engine options";
218+
}
200219
return context;
201220
}
202221

222+
public <T> T getEngineOption(OptionKey<T> key) {
223+
assert engineOptions != null;
224+
if (CompilerDirectives.inInterpreter()) {
225+
return engineOptions.get(key);
226+
} else {
227+
return PythonOptions.getOptionUnrolling(this.engineOptionsStorage, PythonOptions.getEngineOptionKeys(), key);
228+
}
229+
}
230+
203231
@Override
204232
protected OptionDescriptors getOptionDescriptors() {
205233
return PythonOptions.DESCRIPTORS;
@@ -550,4 +578,5 @@ protected void initializeThread(PythonContext context, Thread thread) {
550578
protected void disposeThread(PythonContext context, Thread thread) {
551579
context.disposeThread(thread);
552580
}
581+
553582
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/statement/ExceptNode.java

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.oracle.truffle.api.TruffleException;
5252
import com.oracle.truffle.api.dsl.Cached;
5353
import com.oracle.truffle.api.dsl.CachedContext;
54+
import com.oracle.truffle.api.dsl.CachedLanguage;
5455
import com.oracle.truffle.api.dsl.Fallback;
5556
import com.oracle.truffle.api.dsl.ImportStatic;
5657
import com.oracle.truffle.api.dsl.Specialization;
@@ -140,8 +141,8 @@ public boolean isInstrumentable() {
140141
}
141142

142143
interface EmulateJythonNode {
143-
default boolean emulateJython(PythonContext context) {
144-
return context.getOption(PythonOptions.EmulateJython);
144+
default boolean emulateJython(PythonLanguage language) {
145+
return language.getEngineOption(PythonOptions.EmulateJython);
145146
}
146147
}
147148

@@ -180,10 +181,11 @@ boolean isPythonException(VirtualFrame frame, LazyPythonClass type,
180181
return isSubtype.execute(frame, type, PythonBuiltinClassType.PBaseException);
181182
}
182183

183-
@Specialization(guards = {"emulateJython", "context.getEnv().isHostObject(type)"}, limit = "1")
184+
@Specialization(guards = {"emulateJython(language)", "context.getEnv().isHostObject(type)"})
185+
@SuppressWarnings("unused")
184186
boolean isJavaException(@SuppressWarnings("unused") VirtualFrame frame, Object type,
185-
@CachedContext(PythonLanguage.class) PythonContext context,
186-
@SuppressWarnings("unused") @Cached("emulateJython(context)") boolean emulateJython) {
187+
@CachedLanguage PythonLanguage language,
188+
@CachedContext(PythonLanguage.class) PythonContext context) {
187189
Object hostType = context.getEnv().asHostObject(type);
188190
return hostType instanceof Class && Throwable.class.isAssignableFrom((Class<?>) hostType);
189191
}
@@ -228,31 +230,34 @@ boolean matchPythonSingle(VirtualFrame frame, PException e, Object clause,
228230
return isSubtype.execute(frame, plib.getLazyPythonClass(e.getExceptionObject()), clause);
229231
}
230232

231-
@Specialization(guards = {"emulateJython", "context.getEnv().isHostException(e)", "context.getEnv().isHostObject(clause)"}, limit = "1")
233+
@Specialization(guards = {"emulateJython(language)", "context.getEnv().isHostException(e)", "context.getEnv().isHostObject(clause)"})
234+
@SuppressWarnings("unused")
232235
boolean matchJava(VirtualFrame frame, Throwable e, Object clause,
233236
@Cached ValidExceptionNode isValidException,
234-
@CachedContext(PythonLanguage.class) PythonContext context,
235-
@SuppressWarnings("unused") @Cached("emulateJython(context)") boolean emulateJython) {
237+
@CachedLanguage PythonLanguage language,
238+
@CachedContext(PythonLanguage.class) PythonContext context) {
236239
raiseIfNoException(frame, clause, isValidException);
237240
// cast must succeed due to ValidExceptionNode above
238241
Class<?> javaClause = (Class<?>) context.getEnv().asHostObject(clause);
239242
Throwable hostException = context.getEnv().asHostException(e);
240243
return javaClause.isInstance(hostException);
241244
}
242245

243-
@Specialization(guards = {"emulateJython", "context.getEnv().isHostObject(clause)"}, limit = "1")
246+
@Specialization(guards = {"emulateJython(language)", "context.getEnv().isHostObject(clause)"})
247+
@SuppressWarnings("unused")
244248
boolean doNotMatchPython(VirtualFrame frame, @SuppressWarnings("unused") PException e, Object clause,
245-
@SuppressWarnings("unused") @CachedContext(PythonLanguage.class) PythonContext context,
246-
@SuppressWarnings("unused") @Cached("emulateJython(context)") boolean emulateJython,
249+
@CachedLanguage PythonLanguage language,
250+
@CachedContext(PythonLanguage.class) PythonContext context,
247251
@Cached ValidExceptionNode isValidException) {
248252
raiseIfNoException(frame, clause, isValidException);
249253
return false;
250254
}
251255

252-
@Specialization(guards = {"emulateJython", "context.getEnv().isHostException(e)"}, limit = "1")
256+
@Specialization(guards = {"emulateJython(language)", "context.getEnv().isHostException(e)"})
257+
@SuppressWarnings("unused")
253258
boolean doNotMatchJava(VirtualFrame frame, @SuppressWarnings("unused") Throwable e, LazyPythonClass clause,
254-
@SuppressWarnings("unused") @CachedContext(PythonLanguage.class) PythonContext context,
255-
@SuppressWarnings("unused") @Cached("emulateJython(context)") boolean emulateJython,
259+
@CachedLanguage PythonLanguage language,
260+
@CachedContext(PythonLanguage.class) PythonContext context,
256261
@Cached ValidExceptionNode isValidException) {
257262
raiseIfNoException(frame, clause, isValidException);
258263
return false;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@
9494
import com.oracle.truffle.api.TruffleLogger;
9595
import com.oracle.truffle.api.frame.VirtualFrame;
9696
import com.oracle.truffle.api.interop.UnsupportedMessageException;
97-
import com.oracle.truffle.api.nodes.ExplodeLoop;
9897
import com.oracle.truffle.api.profiles.BranchProfile;
9998
import com.oracle.truffle.api.utilities.CyclicAssumption;
10099

@@ -262,25 +261,10 @@ public <T> T getOption(OptionKey<T> key) {
262261
if (CompilerDirectives.inInterpreter()) {
263262
return getEnv().getOptions().get(key);
264263
} else {
265-
return getOptionUnrolling(key);
264+
return PythonOptions.getOptionUnrolling(this.optionValues, PythonOptions.getOptionKeys(), key);
266265
}
267266
}
268267

269-
@ExplodeLoop
270-
@SuppressWarnings("unchecked")
271-
private <T> T getOptionUnrolling(OptionKey<T> key) {
272-
OptionKey<?>[] optionKeys = PythonOptions.getOptionKeys();
273-
CompilerAsserts.partialEvaluationConstant(optionKeys);
274-
for (int i = 0; i < optionKeys.length; i++) {
275-
CompilerAsserts.partialEvaluationConstant(optionKeys[i]);
276-
if (optionKeys[i] == key) {
277-
return (T) optionValues[i];
278-
}
279-
}
280-
CompilerDirectives.transferToInterpreterAndInvalidate();
281-
throw new IllegalStateException("Using Python options with a non-Python option key");
282-
}
283-
284268
public PythonLanguage getLanguage() {
285269
return language;
286270
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonOptions.java

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
import java.lang.annotation.Target;
3232
import java.lang.reflect.Field;
3333
import java.util.ArrayList;
34+
import java.util.HashMap;
3435
import java.util.List;
36+
import java.util.Map;
3537

3638
import org.graalvm.options.OptionCategory;
3739
import org.graalvm.options.OptionDescriptor;
@@ -42,10 +44,12 @@
4244

4345
import com.oracle.graal.python.PythonLanguage;
4446
import com.oracle.truffle.api.CompilerAsserts;
47+
import com.oracle.truffle.api.CompilerDirectives;
4548
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
4649
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
4750
import com.oracle.truffle.api.Option;
4851
import com.oracle.truffle.api.TruffleLanguage.Env;
52+
import com.oracle.truffle.api.nodes.ExplodeLoop;
4953

5054
/**
5155
* The options for Python. Note that some options have an effect on the AST structure, and thus must
@@ -231,6 +235,13 @@ public static OptionKey<?>[] getOptionKeys() {
231235
return OPTION_KEYS;
232236
}
233237

238+
/**
239+
* A CompilationFinal array of engine option keys defined here. Do not modify!
240+
*/
241+
public static OptionKey<?>[] getEngineOptionKeys() {
242+
return ENGINE_OPTION_KEYS;
243+
}
244+
234245
/**
235246
* Copy values into an array for compilation final storage and unrolling lookup.
236247
*/
@@ -242,6 +253,33 @@ public static Object[] createOptionValuesStorage(Env env) {
242253
return values;
243254
}
244255

256+
public static Object[] createEngineOptionValuesStorage(Env env) {
257+
Object[] values = new Object[ENGINE_OPTION_KEYS.length];
258+
for (int i = 0; i < ENGINE_OPTION_KEYS.length; i++) {
259+
values[i] = env.getOptions().get(ENGINE_OPTION_KEYS[i]);
260+
}
261+
return values;
262+
}
263+
264+
public static OptionValues createEngineOptions(Env env) {
265+
return new EngineOptionValues(env.getOptions());
266+
}
267+
268+
@ExplodeLoop
269+
@SuppressWarnings("unchecked")
270+
public static <T> T getOptionUnrolling(Object[] optionValuesStorage, OptionKey<?>[] optionKeys, OptionKey<T> key) {
271+
assert optionValuesStorage.length == optionKeys.length;
272+
CompilerAsserts.partialEvaluationConstant(optionKeys);
273+
for (int i = 0; i < optionKeys.length; i++) {
274+
CompilerAsserts.partialEvaluationConstant(optionKeys[i]);
275+
if (optionKeys[i] == key) {
276+
return (T) optionValuesStorage[i];
277+
}
278+
}
279+
CompilerDirectives.transferToInterpreterAndInvalidate();
280+
throw new IllegalStateException("Using Python options with a non-Python option key");
281+
}
282+
245283
/**
246284
* Check if the options set in the {@code first} and {@code second} set are compatible, i.e,
247285
* there are no Python per-engine options in these sets that differ.
@@ -297,4 +335,54 @@ private static String[] splitString(String str, String sep) {
297335
@Target(ElementType.FIELD)
298336
@interface EngineOption {
299337
}
338+
339+
private static final class EngineOptionValues implements OptionValues {
340+
341+
private final Map<OptionKey<?>, Object> engineOptions = new HashMap<>();
342+
343+
EngineOptionValues(OptionValues contextOptions) {
344+
for (OptionKey<?> engineKey : ENGINE_OPTION_KEYS) {
345+
if (contextOptions.hasBeenSet(engineKey)) {
346+
engineOptions.put(engineKey, contextOptions.get(engineKey));
347+
}
348+
}
349+
}
350+
351+
public OptionDescriptors getDescriptors() {
352+
throw new UnsupportedOperationException();
353+
}
354+
355+
@Override
356+
public boolean equals(Object obj) {
357+
if (!(obj instanceof EngineOptionValues)) {
358+
return false;
359+
}
360+
EngineOptionValues other = (EngineOptionValues) obj;
361+
return engineOptions.equals(other.engineOptions);
362+
}
363+
364+
@Override
365+
public int hashCode() {
366+
return engineOptions.hashCode();
367+
}
368+
369+
@SuppressWarnings("unchecked")
370+
public <T> T get(OptionKey<T> optionKey) {
371+
if (engineOptions.containsKey(optionKey)) {
372+
return (T) engineOptions.get(optionKey);
373+
} else {
374+
return optionKey.getDefaultValue();
375+
}
376+
}
377+
378+
public boolean hasBeenSet(OptionKey<?> optionKey) {
379+
return engineOptions.containsKey(optionKey);
380+
}
381+
382+
@SuppressWarnings("deprecation")
383+
public <T> void set(OptionKey<T> optionKey, T value) {
384+
throw new UnsupportedOperationException();
385+
}
386+
387+
}
300388
}

0 commit comments

Comments
 (0)