diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_global_statement.py b/graalpython/com.oracle.graal.python.test/src/tests/test_global_statement.py index 0f8b8a82ed..97188b2576 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_global_statement.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/test_global_statement.py @@ -1,4 +1,4 @@ -# Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2020, 2026, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # The Universal Permissive License (UPL), Version 1.0 @@ -37,11 +37,142 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +import builtins +import types import unittest class BasicTests(unittest.TestCase): + def make_global_reader(self, name="value", **values): + module = types.ModuleType("global_read_test") + module.__dict__.update(values) + exec(f"def read():\n return {name}\n", module.__dict__) + return module, module.read + + def warm_global_reader(self, read, expected): + # Exercise the cached interpreter, including its quickened LOAD_GLOBAL. + for _ in range(100): + self.assertIs(read(), expected) + + def test_cached_global_reassignment(self): + module, read = self.make_global_reader(value=object()) + self.warm_global_reader(read, module.value) + for value in (42, 3.5, None, object()): + module.value = value + self.warm_global_reader(read, value) + value = object() + module.__dict__["value"] = value + self.warm_global_reader(read, value) + + def test_cached_global_delete_and_reinsert(self): + module, read = self.make_global_reader(value=object()) + self.warm_global_reader(read, module.value) + del module.value + with self.assertRaises(NameError): + read() + module.value = object() + self.warm_global_reader(read, module.value) + + def test_cached_global_storage_replacement(self): + for operation in ("clear", "non_string_key", "update_non_string_key"): + with self.subTest(operation=operation): + module, read = self.make_global_reader(value=object()) + self.warm_global_reader(read, module.value) + namespace = module.__dict__ + if operation == "clear": + namespace.clear() + elif operation == "non_string_key": + namespace[42] = "force general storage" + else: + namespace.update({42: "force general storage"}) + value = object() + namespace["value"] = value + self.warm_global_reader(read, value) + del namespace["value"] + with self.assertRaises(NameError): + read() + + def test_cached_global_shared_code_different_globals(self): + module, read = self.make_global_reader(value=object()) + self.warm_global_reader(read, module.value) + other = types.ModuleType("other_globals") + other.value = object() + other_read = types.FunctionType(read.__code__, other.__dict__) + plain_value = object() + plain_read = types.FunctionType(read.__code__, {"value": plain_value}) + for _ in range(100): + self.assertIs(other_read(), other.value) + self.assertIs(read(), module.value) + self.assertIs(plain_read(), plain_value) + + def test_cached_builtin_shadow_and_delete(self): + module, read = self.make_global_reader("len") + self.warm_global_reader(read, builtins.len) + module.len = object() + self.warm_global_reader(read, module.len) + del module.len + self.warm_global_reader(read, builtins.len) + + def test_cached_global_falls_back_to_builtin(self): + module, read = self.make_global_reader("len", len=object()) + self.warm_global_reader(read, module.len) + del module.len + self.warm_global_reader(read, builtins.len) + + def test_cached_builtin_storage_replacement(self): + for clear in (False, True): + with self.subTest(clear=clear): + module, read = self.make_global_reader("len") + self.warm_global_reader(read, builtins.len) + namespace = module.__dict__ + if clear: + namespace.clear() + else: + namespace[42] = "force general storage" + value = object() + namespace["len"] = value + self.warm_global_reader(read, value) + + def test_cached_builtin_shared_code_different_globals(self): + module, read = self.make_global_reader("len") + self.warm_global_reader(read, builtins.len) + other = types.ModuleType("other_globals") + other.len = object() + other_read = types.FunctionType(read.__code__, other.__dict__) + plain_value = object() + plain_read = types.FunctionType(read.__code__, {"len": plain_value}) + for _ in range(100): + self.assertIs(other_read(), other.len) + self.assertIs(read(), builtins.len) + self.assertIs(plain_read(), plain_value) + + def test_builtin_after_global_deleted_before_first_read(self): + module, read = self.make_global_reader("len", len=object()) + # A deleted property can remain in the shape with a NO_VALUE value. + del module.len + self.warm_global_reader(read, builtins.len) + module.len = object() + self.warm_global_reader(read, module.len) + + def test_cached_builtin_reassignment_and_deletion(self): + name = "_global_read_test_builtin" + self.assertFalse(hasattr(builtins, name)) + module, read = self.make_global_reader(name) + try: + for _ in range(2): + value = object() + setattr(builtins, name, value) + self.warm_global_reader(read, value) + delattr(builtins, name) + with self.assertRaises(NameError): + read() + module.__dict__[name] = object() + self.warm_global_reader(read, module.__dict__[name]) + finally: + if hasattr(builtins, name): + delattr(builtins, name) + def test_in_local(self): loc = {} glob = {} diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_graalpy_opts.py b/graalpython/com.oracle.graal.python.test/src/tests/test_graalpy_opts.py index fca6a2660d..fbb037d5dc 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_graalpy_opts.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/test_graalpy_opts.py @@ -63,7 +63,10 @@ def tester(): for _ in range(5): assert tester() == 3 - assert_contains_bytecode(tester, "ReadGlobal$ReadBuiltinFastPath") + if __graalpython__.is_single_context: + assert_contains_bytecode(tester, "ReadGlobal$ReadBuiltinFastPath") + else: + assert_contains_bytecode(tester, "ReadGlobal$ReadBuiltinFromStorage") def test_read_global_quickening_global(): @@ -72,7 +75,10 @@ def tester(): for _ in range(5): assert tester() is sys - assert_contains_bytecode(tester, "ReadGlobal$ReadGlobalFastPath") + if __graalpython__.is_single_context: + assert_contains_bytecode(tester, "ReadGlobal$ReadGlobalFastPath") + else: + assert_contains_bytecode(tester, "ReadGlobal$ReadGlobalFromStorage") def test_get_attr_quickening_module(): diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadBuiltinNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadBuiltinNode.java index 517fa98d45..6d26023d40 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadBuiltinNode.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadBuiltinNode.java @@ -98,6 +98,7 @@ Object returnBuiltinFromConstantModule(TruffleString attributeId, @Cached("builtins.getShape()") Shape cachedShape, @Cached(value = "getPropertyGetterWithFinalAssumption(cachedShape, attributeId)", neverDefault = false) PropertyGetter getter, @Bind("getterGet(getter, builtins)") Object value) { + // Note: this is inlined in ReadGlobalOrBuiltinNode, keep in sync return value; } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadGlobalOrBuiltinNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadGlobalOrBuiltinNode.java index 3234ff7a8d..361fbe676d 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadGlobalOrBuiltinNode.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadGlobalOrBuiltinNode.java @@ -31,13 +31,18 @@ import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage; import com.oracle.graal.python.builtins.objects.dict.PDict; import com.oracle.graal.python.builtins.objects.function.PArguments; +import com.oracle.graal.python.builtins.objects.module.PythonModule; import com.oracle.graal.python.builtins.objects.object.PythonObject; import com.oracle.graal.python.lib.PyObjectGetItem.PyObjectGetItemOrNull; +import com.oracle.graal.python.nodes.BuiltinNames; import com.oracle.graal.python.nodes.ErrorMessages; import com.oracle.graal.python.nodes.PGuards; +import com.oracle.graal.python.nodes.PNodeWithContext; import com.oracle.graal.python.nodes.PRaiseNode; import com.oracle.graal.python.nodes.attributes.ReadAttributeFromPythonObjectNode; +import com.oracle.graal.python.runtime.PythonContext; import com.oracle.graal.python.runtime.exception.PException; +import com.oracle.graal.python.util.PythonUtils; import com.oracle.truffle.api.CompilerAsserts; import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff; @@ -53,10 +58,12 @@ import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.ImportStatic; import com.oracle.truffle.api.dsl.NeverDefault; +import com.oracle.truffle.api.dsl.NonIdempotent; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.Frame; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.Node; +import com.oracle.truffle.api.object.PropertyGetter; import com.oracle.truffle.api.object.Shape; import com.oracle.truffle.api.profiles.InlinedBranchProfile; import com.oracle.truffle.api.strings.TruffleString; @@ -65,7 +72,7 @@ @GenerateInline(false) // footprint reduction 48 -> 30 @Proxyable(storeBytecodeIndex = false, allowUncached = true) @ConstantOperand(type = TruffleString.class) -@ImportStatic(PGuards.class) +@ImportStatic({PGuards.class, PNodeWithContext.class, PythonUtils.class}) public abstract class ReadGlobalOrBuiltinNode extends Node { public abstract Object execute(VirtualFrame frame, TruffleString name); @@ -87,6 +94,22 @@ public static ReadGlobalOrBuiltinNode getUncached() { return ReadGlobalOrBuiltinNodeGen.getUncached(); } + /** + * If globals are a dictionary owned by {@link PythonModule}, then using the shape flag + * {@link PythonObject#HAS_MATERIALIZED_DICT} as part of shape check, we can detect when the dictionary storage + * of that module has changed, and we must invalidate our cache. This allows us to check only identity of + * the globals object and its shape and in the runtime guards avoid the pointer chasing done in this method, + * which should be used only at specialization time. + */ + public static PythonModule getGlobalsOwner(VirtualFrame frame) { + CompilerAsserts.neverPartOfCompilation(); + Object obj = PArguments.getGlobals(frame); + if (obj instanceof PDict dict && dict.getDictStorage() instanceof DynamicObjectStorage dom && dom.getStore() instanceof PythonModule module) { + return module; + } + return null; + } + public static Shape getGlobalsStorageShape(VirtualFrame frame) { Object obj = PArguments.getGlobals(frame); if (obj instanceof PDict dict && dict.getDictStorage() instanceof DynamicObjectStorage dom) { @@ -107,9 +130,41 @@ public static Shape getGlobalsStorageShapeIfPropMissing(VirtualFrame frame, Truf } @ForceQuickening - @Specialization(guards = {"cachedGlobalsShape != null", "cachedGlobalsShape == getGlobalsStorageShape(frame)"}, // + @Specialization(guards = { + /* static: */ "isSingleContext(inliningTarget)", "globalsOwner != null", "globalsShape != null", // + /* static: */ "!hasMaterializedDict(globalsShape)", "builtinGetter != null", // + /* dynamic: */ "getGlobals(frame) == cachedGlobals", "globalsShape == getGlobalsOwnerShape(globalsOwner)", // + /* dynamic: */ "getterAccepts(builtinGetter, builtins)", "!isNoValue(result)"}, // excludeForUncached = true, limit = "1") public static Object readBuiltinFastPath(VirtualFrame frame, TruffleString attributeId, + @Bind Node inliningTarget, + @Cached("getGlobals(frame)") Object cachedGlobals, + @Cached("getGlobalsOwner(frame)") PythonModule globalsOwner, + @Cached("getGlobalsStorageShapeIfPropMissing(frame, attributeId)") Shape globalsShape, + @Cached("getBuiltins(inliningTarget)") PythonModule builtins, + @Cached("getBuiltinGetter(builtins, attributeId)") PropertyGetter builtinGetter, + @Bind("getValue(builtins, builtinGetter)") Object result) { + // Note: this is inlined version of ReadBuiltinNode#returnBuiltinFromConstantModule, keep in sync + // Both shape checks also guard against replacement of the module-backed dict storages. + return result; + } + + public static PythonModule getBuiltins(Node node) { + CompilerAsserts.neverPartOfCompilation(); + PythonContext context = PythonContext.get(node); + return context.isInitialized() ? context.getBuiltins() : context.lookupBuiltinModule(BuiltinNames.T_BUILTINS); + } + + public static PropertyGetter getBuiltinGetter(PythonModule builtins, TruffleString name) { + // The getter retains the shape, so no separate cached builtins shape is needed. + Shape shape = builtins.getShape(); + return PGuards.hasMaterializedDict(shape) ? null : PythonUtils.getPropertyGetterWithFinalAssumption(shape, name); + } + + @ForceQuickening + @Specialization(guards = {"cachedGlobalsShape != null", "cachedGlobalsShape == getGlobalsStorageShape(frame)"}, // + replaces = "readBuiltinFastPath", excludeForUncached = true, limit = "1") + public static Object readBuiltinFromStorage(VirtualFrame frame, TruffleString attributeId, @Cached("getGlobalsStorageShapeIfPropMissing(frame, attributeId)") Shape cachedGlobalsShape, @Shared("readFromBuiltinsNode") @Cached ReadBuiltinNode readFromBuiltinsNode) { return readFromBuiltinsNode.execute(attributeId); @@ -123,16 +178,59 @@ public static Object readFastFromGlobalStore(VirtualFrame frame, TruffleString n return PNone.NO_VALUE; } + @NonIdempotent + public static Object getValue(PythonModule m, PropertyGetter getter) { + assert m.checkDictFlags(); + return getter.get(m); + } + + @NonIdempotent + public static Object getGlobals(VirtualFrame frame) { + return PArguments.getGlobals(frame); + } + + @NonIdempotent + public static Shape getGlobalsOwnerShape(PythonModule module) { + return module.getShape(); + } + + @NonIdempotent + public static boolean getterAccepts(PropertyGetter getter, PythonModule module) { + return getter.accepts(module); + } + @ForceQuickening - @Specialization(guards = "!isNoValue(result)", replaces = "readBuiltinFastPath", excludeForUncached = true, limit = "1") + @Specialization(guards = { + /* static: */ "isSingleContext(inliningTarget)", "globalsOwner != null", "!hasMaterializedDict(globalsShape)", "getter != null", // + /* dynamic: */ "getGlobals(frame) == cachedGlobals", "getterAccepts(getter, globalsOwner)", "!isNoValue(result)"}, // + replaces = "readBuiltinFromStorage", excludeForUncached = true, limit = "1") public static Object readGlobalFastPath(VirtualFrame frame, TruffleString attributeId, + @Bind Node inliningTarget, + @Cached("getGlobals(frame)") Object cachedGlobals, + @Cached("getGlobalsOwner(frame)") PythonModule globalsOwner, + @Cached("globalsOwner.getShape()") Shape globalsShape, + @Cached("getPropertyGetterWithFinalAssumption(globalsShape, attributeId)") PropertyGetter getter, + @Bind("getValue(globalsOwner, getter)") Object result) { + CompilerAsserts.partialEvaluationConstant(attributeId); + // since the shape does not have MATERIALIZED_DICT shape, and we do shape check on the owner, + // the dict storage must not have been replaced + assert cachedGlobals instanceof PDict d && // + d.getDictStorage() instanceof DynamicObjectStorage s && // + s.getStore() == globalsOwner; + return result; + } + + @ForceQuickening + @Specialization(guards = "!isNoValue(result)", replaces = "readGlobalFastPath", excludeForUncached = true, limit = "1") + public static Object readGlobalFromStorage(VirtualFrame frame, TruffleString attributeId, @Cached(inline = false) ReadAttributeFromPythonObjectNode readNode, @Bind("readFastFromGlobalStore(frame, attributeId, readNode)") Object result) { return result; } + @ForceQuickening @StoreBytecodeIndex - @Specialization(replaces = {"readBuiltinFastPath", "readGlobalFastPath"}) + @Specialization(replaces = "readGlobalFromStorage") public static Object readGlobalOrBuiltinGeneric(VirtualFrame frame, TruffleString attributeId, @Bind Node inliningTarget, @Shared("readFromBuiltinsNode") @Cached ReadBuiltinNode readFromBuiltinsNode,