From be0015c895201d4633fb64f5046084a05ba19451 Mon Sep 17 00:00:00 2001 From: stepan Date: Thu, 10 Sep 2026 21:52:57 +0200 Subject: [PATCH 1/2] Optimize instance attribute loads/stores using MRO stable lookup assumptions Co-authored-by: Octave Larose --- .../test_mro_lookup_cache_invalidation.py | 18 ++ .../src/tests/test_descr.py | 195 ++++++++++++++++++ .../modules/cext/PythonCextTypeBuiltins.java | 4 + .../objects/common/DynamicObjectStorage.java | 6 +- .../builtins/objects/type/PythonClass.java | 57 ++++- .../objects/type/PythonManagedClass.java | 5 +- .../python/builtins/objects/type/TpSlots.java | 7 + .../attributes/LookupAttributeInMRONode.java | 26 +++ .../ReadAttributeFromObjectNoSideEffects.java | 80 +++++++ .../bytecode_dsl/PBytecodeDSLRootNode.java | 186 +++++++++-------- 10 files changed, 485 insertions(+), 99 deletions(-) create mode 100644 graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromObjectNoSideEffects.java diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_mro_lookup_cache_invalidation.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_mro_lookup_cache_invalidation.py index 776c525a6a..d7f1d20f67 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_mro_lookup_cache_invalidation.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_mro_lookup_cache_invalidation.py @@ -187,6 +187,24 @@ def do_lookup(obj): assert bool(x) is True +def test_pytype_modified_on_python_type_invalidates_instance_attribute_fast_path(): + class PythonType: + pass + + x = PythonType() + x.value = "instance" + + def do_lookup(obj): + return obj.value + + for i in range(10): + assert do_lookup(x) == "instance" + + TypeAttrHelper.changeAttr(PythonType, "value", property(lambda self: "descriptor")) + for i in range(10): + assert do_lookup(x) == "descriptor" + + def test_pytype_modified_after_deleting_special_method_invalidates_slot_lookup(): class PythonBase: def __len__(self): diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_descr.py b/graalpython/com.oracle.graal.python.test/src/tests/test_descr.py index 4722f16c89..c8d533d22e 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/test_descr.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/test_descr.py @@ -37,6 +37,201 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. import gc +import warnings + + +def test_cached_instance_load_after_descriptor_added(): + class C: + pass + + def read(obj): + return obj.attr + + obj = C() + obj.attr = "instance" + for _ in range(20): + assert read(obj) == "instance" + + C.attr = property(lambda self: "descriptor") + assert read(obj) == "descriptor" + + +def test_cached_instance_int_load_after_inherited_descriptor_added(): + class Base: + pass + + class C(Base): + pass + + def read(obj): + return obj.attr + 1 + + obj = C() + obj.attr = 41 + for _ in range(20): + assert read(obj) == 42 + + Base.attr = property(lambda self: 99) + assert read(obj) == 100 + + +def test_cached_instance_load_after_getattribute_added(): + class C: + pass + + def read(obj): + return obj.attr + + obj = C() + obj.attr = "instance" + for _ in range(20): + assert read(obj) == "instance" + + C.__getattribute__ = lambda self, name: "override" + assert read(obj) == "override" + + +def test_cached_instance_store_after_descriptor_replaced(): + class C: + def attr(self): + pass + + def write(obj, value): + obj.attr = value + + obj = C() + for value in range(20): + write(obj, value) + assert obj.attr == 19 + + writes = [] + C.attr = property(lambda self: "descriptor", lambda self, value: writes.append(value)) + write(obj, 42) + assert writes == [42] + assert obj.__dict__["attr"] == 19 + + +def test_cached_instance_store_after_inherited_setattr_added(): + class Base: + pass + + class C(Base): + pass + + def write(obj, value): + obj.attr = value + + obj = C() + for value in range(20): + write(obj, value) + assert obj.attr == 19 + + writes = [] + Base.__setattr__ = lambda self, name, value: writes.append((name, value)) + write(obj, 42) + assert writes == [("attr", 42)] + assert obj.__dict__["attr"] == 19 + + +def _change_base_while_warming_attribute_access(new_base, warmup): + class Base: + pass + + class C(Base): + pass + + obj = C() + obj.attr = 41 + armed = False + + class Meta(type): + def mro(cls): + if armed: + # C's MRO has already changed and invalidated its lookup caches, + # but its slots still come from Base. Specializing here must not + # leave cached attribute accesses valid after the slots change. + warmup(obj) + return super().mro() + + class Child(C, metaclass=Meta): + pass + + armed = True + C.__bases__ = (new_base,) + return obj + + +def test_cached_instance_load_during_bases_change(): + class Override: + def __getattribute__(self, name): + return 99 + + def read(obj): + return obj.attr + + def warmup(obj): + for _ in range(20): + read(obj) + + obj = _change_base_while_warming_attribute_access(Override, warmup) + assert getattr(obj, "attr") == 99 + assert read(obj) == 99 + + +def test_cached_instance_int_load_during_bases_change(): + class Override: + def __getattribute__(self, name): + return 99 + + def read(obj): + return obj.attr + 1 + + def warmup(obj): + for _ in range(20): + read(obj) + + obj = _change_base_while_warming_attribute_access(Override, warmup) + assert getattr(obj, "attr") == 99 + assert read(obj) == 100 + + +def test_cached_instance_store_during_bases_change(): + writes = [] + + class Override: + def __setattr__(self, name, value): + writes.append((name, value)) + + def write(obj, value): + obj.attr = value + + def warmup(obj): + for value in range(20): + write(obj, value) + + obj = _change_base_while_warming_attribute_access(Override, warmup) + writes.clear() + value_before = obj.attr + write(obj, 42) + assert writes == [("attr", 42)] + assert obj.attr == value_before + + +def test_instance_store_with_non_string_class_dict_key(): + # Such a namespace requires a dictionary that cannot be probed by the + # side-effect-free descriptor lookup used when specializing STORE_ATTR. + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RuntimeWarning) + C = type("C", (), {1: None}) + + def write(obj, value): + obj.attr = value + + obj = C() + for value in range(20): + write(obj, value) + assert obj.attr == value + def test_evil_getattribute(): # Variation of a CPython test from test_descr.py diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java index 1fceeeb86e..479a151df7 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTypeBuiltins.java @@ -100,6 +100,7 @@ import com.oracle.graal.python.builtins.objects.getsetdescriptor.GetSetDescriptor; import com.oracle.graal.python.builtins.objects.object.PythonBuiltinObject; import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass; +import com.oracle.graal.python.builtins.objects.type.PythonClass; import com.oracle.graal.python.builtins.objects.type.PythonManagedClass; import com.oracle.graal.python.builtins.objects.type.TpSlots; import com.oracle.graal.python.builtins.objects.type.TypeNodes; @@ -259,14 +260,17 @@ static Object doIt(PythonAbstractClass object, public static void invalidateMroLookup(PythonAbstractClass klass) { // Note: PyType_Modified should not recompute the slots, just invalidate lookup caches PythonAbstractClass[] allSubclasses = GetSubclassesAsArrayNode.executeRecursiveUncached(klass); + PythonClass.invalidateTypeStableAssumption(klass); TpSlots.updateSlotWrappersLookups(klass, allSubclasses); MroSequenceStorage mroStorage = TypeNodes.GetMroStorageNode.executeUncached(klass); mroStorage.lookupChanged(); for (PythonAbstractClass subclass : allSubclasses) { + PythonClass.invalidateTypeStableAssumption(subclass); MroSequenceStorage subClassMroStorage = TypeNodes.GetMroStorageNode.executeUncached(subclass); subClassMroStorage.lookupChanged(); } } + } @CApiBuiltin(ret = Int, args = {Pointer}, call = Ignored) diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/DynamicObjectStorage.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/DynamicObjectStorage.java index 59caca476c..f0c8d0713e 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/DynamicObjectStorage.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/DynamicObjectStorage.java @@ -171,13 +171,17 @@ private static int incrementLen(DynamicObjectStorage self, ReadAttributeFromPyth @GenerateUncached @GenerateInline @GenerateCached(false) - abstract static class GetItemNode extends Node { + public abstract static class GetItemNode extends Node { /** * For builtin strings the {@code keyHash} value is ignored and can be garbage. If the * {@code keyHash} is equal to {@code -1} it will be computed for non-string keys. */ public abstract Object execute(Frame frame, Node inliningTarget, DynamicObjectStorage self, Object key, long keyHash); + public static Object executeSlowPath(DynamicObjectStorage storage, TruffleString key, Object defaultValue) { + return DynamicObject.GetNode.getUncached().execute(storage.store, key, defaultValue); + } + @Specialization static Object string(Node inliningTarget, DynamicObjectStorage self, TruffleString key, @SuppressWarnings("unused") long keyHash, @Shared @Cached ReadAttributeFromPythonObjectNode readKey, diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java index 0ecc15ba82..c33925c6fa 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java @@ -38,6 +38,7 @@ import com.oracle.graal.python.nodes.object.GetClassNode; import com.oracle.graal.python.runtime.GilNode; import com.oracle.graal.python.util.SuppressFBWarnings; +import com.oracle.truffle.api.Assumption; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Bind; import com.oracle.truffle.api.dsl.Cached; @@ -53,6 +54,7 @@ import com.oracle.truffle.api.profiles.InlinedBranchProfile; import com.oracle.truffle.api.source.SourceSection; import com.oracle.truffle.api.strings.TruffleString; +import com.oracle.truffle.api.utilities.CyclicAssumption; /** * Mutable class. @@ -61,6 +63,7 @@ public final class PythonClass extends PythonManagedClass { private static final int MRO_SHAPE_INVALIDATIONS_MAX = 5; + private static final int TYPE_STABLE_INVALIDATIONS_MAX = 100; /** * MroShape is only set if all base classes in MRO have mroShape set. @@ -69,16 +72,58 @@ public final class PythonClass extends PythonManagedClass { private MroShape mroShape; private byte mroShapeInvalidationsCount; + /** Assumption for instance attribute fast paths, which are only used in single-context mode. */ + private final CyclicAssumption typeStableAssumption; + private byte typeStableInvalidationsCount = 0; + public PythonClass(Node location, PythonLanguage lang, Object typeClass, Shape classShape, TruffleString name, Object base, PythonAbstractClass[] baseClasses) { - super(location, lang, typeClass, classShape, null, name, base, baseClasses, null); + this(location, lang, typeClass, classShape, name, true, true, base, baseClasses); } public PythonClass(Node location, PythonLanguage lang, Object typeClass, Shape classShape, TruffleString name, boolean invokeMro, Object base, PythonAbstractClass[] baseClasses) { - super(location, lang, typeClass, classShape, null, name, invokeMro, false, base, baseClasses, null); + this(location, lang, typeClass, classShape, name, invokeMro, false, base, baseClasses); + } + + private PythonClass(Node location, PythonLanguage lang, Object typeClass, Shape classShape, TruffleString name, boolean invokeMro, boolean initDocAttr, Object base, + PythonAbstractClass[] baseClasses) { + super(location, lang, typeClass, classShape, null, name, invokeMro, initDocAttr, base, baseClasses, null); + typeStableAssumption = lang.isSingleContext() ? new CyclicAssumption("Python class stable") : null; } public void setTpSlots(TpSlots tpSlots) { this.tpSlots = tpSlots; + invalidateTypeStableAssumption(); + } + + public Assumption getTypeStableAssumption() { + return typeStableAssumption == null ? Assumption.NEVER_VALID : typeStableAssumption.getAssumption(); + } + + /** + * Invalidates instance attribute fast paths after a type dictionary is changed without going + * through the regular managed attribute update path, for example by {@code PyType_Modified}. + */ + @SuppressFBWarnings(value = "UR_UNINIT_READ_CALLED_FROM_SUPER_CONSTRUCTOR") + @TruffleBoundary + public void invalidateTypeStableAssumption() { + // Also called by attribute initialization in the superclass constructor. + if (typeStableAssumption == null) { + return; + } + byte invalidationsCount = typeStableInvalidationsCount; + if (invalidationsCount < TYPE_STABLE_INVALIDATIONS_MAX) { + typeStableInvalidationsCount = (byte) (invalidationsCount + 1); + typeStableAssumption.invalidate(); + } else { + // Do not create new Assumption, just make sure that the current one is invalid + typeStableAssumption.getAssumption().invalidate(); + } + } + + public static void invalidateTypeStableAssumption(PythonAbstractClass klass) { + if (klass instanceof PythonClass pythonClass) { + pythonClass.invalidateTypeStableAssumption(); + } } @Override @@ -89,6 +134,12 @@ public void setAttribute(TruffleString key, Object value) { invalidateMroShapeSubTypes(); } + @Override + void onAttributeUpdateSelf(TruffleString key, Object value) { + invalidateTypeStableAssumption(); + super.onAttributeUpdateSelf(key, value); + } + @ExportMessage(library = InteropLibrary.class) @SuppressWarnings("static-method") boolean isMetaObject() { @@ -192,10 +243,12 @@ public void setMRO(PythonAbstractClass[] mro) { super.setMRO(mro); mroShape = null; invalidateMroShapeSubTypes(); + invalidateTypeStableAssumption(); } public void setMRO(PythonAbstractClass[] mro, PythonLanguage language) { super.setMRO(mro); + invalidateTypeStableAssumption(); if (!language.isSingleContext()) { mroShape = null; invalidateMroShapeSubTypes(); diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java index 94b2c2c92b..36dd2dd1c9 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java @@ -110,7 +110,8 @@ protected PythonManagedClass(Node location, PythonLanguage lang, Object typeClas unsafeSetSuperClass(baseClasses); } - this.setMRO(ComputeMroNode.doSlowPath(location, this, invokeMro)); + // set field directly to avoid any invalidations, there is no-one that can rely on the MRO yet + methodResolutionOrder = new MroSequenceStorage(name, ComputeMroNode.doSlowPath(location, this, invokeMro)); if (invokeMro) { mroInitialized = true; } @@ -249,7 +250,7 @@ final void onAttributeUpdate(TruffleString key, Object value, PythonAbstractClas /** * Non-recursive part of the {@link #onAttributeUpdate(TruffleString, Object)}. */ - private void onAttributeUpdateSelf(TruffleString key, Object value) { + void onAttributeUpdateSelf(TruffleString key, Object value) { methodResolutionOrder.invalidateFinalAttributeAssumption(key); } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TpSlots.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TpSlots.java index b721f8ff0b..356f6d46b4 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TpSlots.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TpSlots.java @@ -1493,6 +1493,13 @@ public static void fixupSlotDispatchers(PythonClass klass) { @TruffleBoundary public static void updateAllSlots(PythonAbstractClass klass, PythonAbstractClass[] allSubclasses) { updateSlot(klass, SLOTDEFS.entrySet(), allSubclasses); + // MRO recomputation may execute arbitrary Python code before reaching this point. Such + // code can specialize instance attribute accesses while the old slot table is still in + // use. Invalidate again after the update, including when no individual slot changed. + PythonClass.invalidateTypeStableAssumption(klass); + for (PythonAbstractClass subclass : allSubclasses) { + PythonClass.invalidateTypeStableAssumption(subclass); + } } /** diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java index 02c5e58add..7a61d90480 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java @@ -160,6 +160,12 @@ public static LookupAttributeInMRONode createForLookupOfUnmanagedClasses(Truffle return LookupAttributeInMRONodeGen.create(key, true); } + @NeverDefault + public static Object findAttr(PythonBuiltinClassType klass, TruffleString key) { + CompilerAsserts.neverPartOfCompilation(); // Use overload with Python3Core and pass PythonContext.get(node) + return findAttr(PythonContext.get(null), klass, key, ReadAttributeFromPythonObjectNode.getUncached()); + } + @NeverDefault static Object findAttr(Python3Core core, PythonBuiltinClassType klass, TruffleString key) { return findAttr(core, klass, key, ReadAttributeFromPythonObjectNode.getUncached()); @@ -494,4 +500,24 @@ public static Object lookup(TruffleString key, MroSequenceStorage mro, ReadAttri } return PNone.NO_VALUE; } + + /** + * The same as {@link #lookupSlowPath(Object, SlowPath)} except that it does not probe dictionaries + * that may have side effects, if such dictionary is encountered, returns {@code null}. + */ + public static Object lookupSlowPathNoSideEffects(Object klass, TruffleString key) { + CompilerAsserts.neverPartOfCompilation(); + MroSequenceStorage mro = GetMroStorageNode.executeUncached(klass); + for (int i = 0; i < mro.length(); i++) { + Object kls = mro.getPythonClassItemNormalized(i); + Object value = ReadAttributeFromObjectNoSideEffects.executeUncached(kls, key); + if (value == null) { + return null; + } + if (value != PNone.NO_VALUE) { + return value; + } + } + return PNone.NO_VALUE; + } } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromObjectNoSideEffects.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromObjectNoSideEffects.java new file mode 100644 index 0000000000..9145520758 --- /dev/null +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromObjectNoSideEffects.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2026, 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 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.graal.python.nodes.attributes; + +import com.oracle.graal.python.builtins.objects.PNone; +import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject; +import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage; +import com.oracle.graal.python.builtins.objects.common.HashingStorage; +import com.oracle.graal.python.builtins.objects.object.PythonObject; +import com.oracle.graal.python.nodes.object.GetDictIfExistsNode; +import com.oracle.truffle.api.strings.TruffleString; + +/** + * The same as {@link ReadAttributeFromObjectNode} except that it refuses to read from + * potentially side-effecting dictionaries and returns {@code null} in such case. + */ +public abstract class ReadAttributeFromObjectNoSideEffects { + public static Object executeUncached(Object object, TruffleString key) { + HashingStorage dictStorage; + if (object instanceof PythonObject pyObject) { + var dict = GetDictIfExistsNode.getDictUncached(pyObject); + assert pyObject.checkDictFlags(dict); + if (dict == null) { + return ReadAttributeFromPythonObjectNode.executeUncached(pyObject, key, PNone.NO_VALUE); + } + dictStorage = dict.getDictStorage(); + } else if (object instanceof PythonAbstractNativeObject pyNativeObject) { + var dict = GetDictIfExistsNode.getUncached().execute(pyNativeObject); + if (dict == null) { + return PNone.NO_VALUE; + } + dictStorage = dict.getDictStorage(); + } else { + // foreign object or primitive + return PNone.NO_VALUE; + } + if (dictStorage instanceof DynamicObjectStorage domStorage) { + return DynamicObjectStorage.GetItemNode.executeSlowPath(domStorage, key, PNone.NO_VALUE); + } + return null; + } +} diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java index 874704b242..305c13bb32 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java @@ -114,6 +114,8 @@ import com.oracle.graal.python.builtins.objects.set.PSet; import com.oracle.graal.python.builtins.objects.set.SetNodes; import com.oracle.graal.python.builtins.objects.tuple.PTuple; +import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass; +import com.oracle.graal.python.builtins.objects.type.PythonClass; import com.oracle.graal.python.builtins.objects.type.PythonManagedClass; import com.oracle.graal.python.builtins.objects.type.TpSlots; import com.oracle.graal.python.builtins.objects.type.TpSlots.GetObjectSlotsNode; @@ -1942,6 +1944,20 @@ public static Object doStringFastPath(VirtualFrame frame, TruffleString name, Tr return result; } + private static boolean hasObjectOrModuleGetattro(Node inliningTarget, PythonManagedClass klass, InlineWeakValueProfile slotsValueProfile) { + TpSlots slots = slotsValueProfile.execute(inliningTarget, klass.getTpSlots()); + return GetAttribute.hasObjectOrModuleGetattro(slots); + } + + @Idempotent + public static boolean isBuiltinWithObjectOrModuleGetattro(Shape cachedShape) { + return cachedShape.getDynamicType() instanceof PythonBuiltinClassType type && GetAttribute.hasObjectOrModuleGetattro(type.getSlots()); + } + + public static PythonManagedClass getManagedClassOrNull(Shape cachedShape) { + return cachedShape.getDynamicType() instanceof PythonManagedClass managedClass ? managedClass : null; + } + @ForceQuickening @Specialization(guards = { "!hasMaterializedDict(cachedShape)", "managedClass != null || isBuiltinWithObjectOrModuleGetattro(cachedShape)", // @@ -1963,7 +1979,7 @@ public static Object doFastPath(VirtualFrame frame, static Object getMethodFastPath(PythonObject obj, TruffleString name, Node inliningTarget, PythonManagedClass managedClass, Shape cachedShape, PropertyGetter cachedPropertyGetter, InlineWeakValueProfile slotsValueProfile, InlinedBranchProfile hasInstanceValueBranchProfile, LookupAttributeInMRONode.CachedKeyFastPath getMethod) { if (managedClass != null) { - if (!GetAttribute.hasObjectOrModuleGetattro(inliningTarget, managedClass, slotsValueProfile)) { + if (!hasObjectOrModuleGetattro(inliningTarget, managedClass, slotsValueProfile)) { return null; } } @@ -2126,86 +2142,66 @@ static Object doType(VirtualFrame frame, TruffleString key, PythonManagedClass r return loadTypeInstanceValue(frame, inliningTarget, receiver, getObjectSlotsNode, callSlotDescrGet, getter, hasNonDescriptorValueProfile); } - // The convention is that if klass is null, then the object is assumed to be of a builtin - // type that has the object's or module's tp_getattro - we do not need to recheck that - // dynamically - public static Object loadInstanceValue(Node inliningTarget, PythonObject object, PythonManagedClass klass, - TruffleString key, LookupAttributeInMRONode.CachedKeyFastPath getDesc, Shape cachedShape, PropertyGetter cachedPropertyGetter, - InlineWeakValueProfile slotsValueProfile) throws FastPathBailoutException { - checkCanLoadInstanceValue(inliningTarget, object, klass, key, getDesc, cachedShape, slotsValueProfile); - Object value = cachedPropertyGetter.get(object); - if (value == PNone.NO_VALUE) { - throw FastPathBailoutException.INSTANCE; - } - return value; - } - - public static int loadInstanceValueInt(Node inliningTarget, PythonObject object, PythonManagedClass klass, - TruffleString key, LookupAttributeInMRONode.CachedKeyFastPath getDesc, Shape cachedShape, PropertyGetter cachedPropertyGetter, - InlineWeakValueProfile slotsValueProfile) throws FastPathBailoutException, UnexpectedResultException { - checkCanLoadInstanceValue(inliningTarget, object, klass, key, getDesc, cachedShape, slotsValueProfile); - return getIntValue(cachedPropertyGetter, object); + private static boolean hasObjectOrModuleGetattro(TpSlots slots) { + return slots.tp_getattro() == ObjectBuiltins.SLOTS.tp_getattro() || slots.tp_getattro() == ModuleBuiltins.SLOTS.tp_getattro(); } - private static void checkCanLoadInstanceValue(Node inliningTarget, PythonObject object, PythonManagedClass klass, - TruffleString key, LookupAttributeInMRONode.CachedKeyFastPath getDesc, Shape cachedShape, - InlineWeakValueProfile slotsValueProfile) throws FastPathBailoutException { - if (klass == null || hasObjectOrModuleGetattro(inliningTarget, klass, slotsValueProfile)) { - Object descr = getDesc.execute(inliningTarget, cachedShape.getDynamicType(), key); - if (descr == PNone.NO_VALUE) { - assert object.checkDictFlags(); - return; - } + public static boolean canBypassDescriptorLookup(Shape cachedShape, TruffleString key) { + assert PythonContext.get(null).ownsGil(); // otherwise re-check cachedMroLookupVersion, cachedShape + CompilerAsserts.neverPartOfCompilation(); + Object type = cachedShape.getDynamicType(); + if (type instanceof PythonBuiltinClass pbc) { + type = pbc.getType(); } - throw FastPathBailoutException.INSTANCE; - } - - private static boolean hasObjectOrModuleGetattro(Node inliningTarget, PythonManagedClass klass, InlineWeakValueProfile slotsValueProfile) { - TpSlots slots = slotsValueProfile.execute(inliningTarget, klass.getTpSlots()); - return hasObjectOrModuleGetattro(slots); + if (type instanceof PythonBuiltinClassType pbct) { + // builtins cannot change + return hasObjectOrModuleGetattro(pbct.getSlots()) && LookupAttributeInMRONode.findAttr(pbct, key) == PNone.NO_VALUE; + } else if (type instanceof PythonClass klass) { + return hasObjectOrModuleGetattro(klass.getTpSlots()) && LookupAttributeInMRONode.lookupSlowPathNoSideEffects(klass, key) == PNone.NO_VALUE; + } + return false; } - private static boolean hasObjectOrModuleGetattro(TpSlots slots) { - return slots.tp_getattro() == ObjectBuiltins.SLOTS.tp_getattro() || slots.tp_getattro() == ModuleBuiltins.SLOTS.tp_getattro(); + public static PythonClass getPythonClassOrNull(Shape cachedShape) { + CompilerAsserts.neverPartOfCompilation(); + return cachedShape.getDynamicType() instanceof PythonClass kls ? kls : null; } - public static PythonManagedClass getManagedClassOrNull(Shape cachedShape) { - return cachedShape.getDynamicType() instanceof PythonManagedClass managedClass ? managedClass : null; + public static Assumption getTypeStableAssumption(PythonClass klass) { + return klass == null ? Assumption.ALWAYS_VALID : klass.getTypeStableAssumption(); } - @Idempotent - public static boolean isBuiltinWithObjectOrModuleGetattro(Shape cachedShape) { - return cachedShape.getDynamicType() instanceof PythonBuiltinClassType type && hasObjectOrModuleGetattro(type.getSlots()); - } - - @StoreBytecodeIndex // looking up attribute in MRO may have side effects @Specialization(guards = { - "!hasMaterializedDict(cachedShape)", "managedClass != null || isBuiltinWithObjectOrModuleGetattro(cachedShape)", // - "getter != null", "getter.accepts(receiver)"}, // - rewriteOn = {FastPathBailoutException.class, UnexpectedResultException.class}, limit = "3") + /* static checks: */ "noDescriptor", "!hasMaterializedDict(cachedShape)", "getter != null", + /* dynamic checks: */ "getter.accepts(receiver)"}, // + assumptions = "typeStableAssumption", // + rewriteOn = {FastPathBailoutException.class, UnexpectedResultException.class}, limit = "3", excludeForUncached = true) static int doInstanceValueInt(TruffleString key, PythonObject receiver, @Bind Node inliningTarget, @Cached("receiver.getShape()") Shape cachedShape, - @Cached("getManagedClassOrNull(cachedShape)") PythonManagedClass managedClass, - @Cached("getPropertyGetterWithFinalAssumption(cachedShape, key)") PropertyGetter getter, - @Exclusive @Cached LookupAttributeInMRONode.CachedKeyFastPath getDesc, - @Exclusive @Cached InlineWeakValueProfile slotsValueProfile) throws FastPathBailoutException, UnexpectedResultException { - return loadInstanceValueInt(inliningTarget, receiver, managedClass, key, getDesc, cachedShape, getter, slotsValueProfile); + @Cached("canBypassDescriptorLookup(cachedShape, key)") boolean noDescriptor, + @Cached("getPythonClassOrNull(cachedShape)") PythonClass cachedPythonClass, + @Cached("getTypeStableAssumption(cachedPythonClass)") Assumption typeStableAssumption, + @Cached("getPropertyGetterWithFinalAssumption(cachedShape, key)") PropertyGetter getter) throws FastPathBailoutException, UnexpectedResultException { + assert cachedPythonClass == null || PythonLanguage.get(null).isSingleContext(); + return getIntValue(getter, receiver); } @ForceQuickening @Specialization(guards = { - "!hasMaterializedDict(cachedShape)", "managedClass != null || isBuiltinWithObjectOrModuleGetattro(cachedShape)", // - "getter != null", "getter.accepts(receiver)"}, // - replaces = "doInstanceValueInt", rewriteOn = FastPathBailoutException.class, limit = "3") + /* static checks: */ "noDescriptor", "!hasMaterializedDict(cachedShape)", "getter != null", // + /* dynamic checks: */ "getter.accepts(receiver)"}, // + assumptions = "typeStableAssumption", // + replaces = "doInstanceValueInt", rewriteOn = FastPathBailoutException.class, limit = "3", excludeForUncached = true) static Object doInstanceValue(TruffleString key, PythonObject receiver, @Bind Node inliningTarget, @Cached("receiver.getShape()") Shape cachedShape, - @Cached("getManagedClassOrNull(cachedShape)") PythonManagedClass managedClass, - @Cached("getPropertyGetterWithFinalAssumption(cachedShape, key)") PropertyGetter getter, - @Exclusive @Cached LookupAttributeInMRONode.CachedKeyFastPath getDesc, - @Exclusive @Cached InlineWeakValueProfile slotsValueProfile) throws FastPathBailoutException { - return loadInstanceValue(inliningTarget, receiver, managedClass, key, getDesc, cachedShape, getter, slotsValueProfile); + @Cached("canBypassDescriptorLookup(cachedShape, key)") boolean noDescriptor, + @Cached("getPythonClassOrNull(cachedShape)") PythonClass cachedPythonClass, + @Cached("getTypeStableAssumption(cachedPythonClass)") Assumption typeStableAssumption, + @Cached("getPropertyGetterWithFinalAssumption(cachedShape, key)") PropertyGetter getter) throws FastPathBailoutException { + assert cachedPythonClass == null || PythonLanguage.get(null).isSingleContext(); + return getValue(getter, receiver); } @Specialization(excludeForUncached = true, replaces = {"doModule", "doInstanceValue", "doType"}) @@ -2229,37 +2225,42 @@ public static Object doItUncached(VirtualFrame frame, TruffleString key, Object @Operation(storeBytecodeIndex = true) @ConstantOperand(type = TruffleString.class) - @ImportStatic(PGuards.class) + @ImportStatic({PGuards.class, GetAttribute.class}) public static final class SetAttribute { - @NonIdempotent - public static boolean canStoreInstanceValue(Node inliningTarget, TruffleString key, PythonManagedClass managedClass, Shape cachedShape, LookupAttributeInMRONode.CachedKeyFastPath getDesc, - GetObjectSlotsNode getDescSlotsNode, InlineWeakValueProfile slotsValueProfile) { - if (managedClass == null || slotsValueProfile.execute(inliningTarget, managedClass.getTpSlots()).tp_setattro() == ObjectBuiltins.SLOTS.tp_setattro()) { - Object descr = getDesc.execute(inliningTarget, cachedShape.getDynamicType(), key); - return descr != null && (descr == PNone.NO_VALUE || getDescSlotsNode.execute(inliningTarget, descr).tp_descr_set() == null); - } - return false; + private static boolean hasObjectSetattro(TpSlots slots) { + return slots.tp_setattro() == ObjectBuiltins.SLOTS.tp_setattro(); } - public static boolean canSkipDescriptorCheck(Shape cachedShape, TruffleString key) { - if (cachedShape.getDynamicType() instanceof PythonBuiltinClassType type && type.getSlots().tp_setattro() == ObjectBuiltins.SLOTS.tp_setattro()) { - Object descr = LookupAttributeInMRONode.Dynamic.getUncached().execute(type, key); - if (descr == PNone.NO_VALUE) { - return true; + public static boolean canBypassDescriptorLookupForStore(Shape cachedShape, TruffleString key) { + assert PythonContext.get(null).ownsGil(); // otherwise: re-check cachedShape, cachedMroLookupVersion + CompilerAsserts.neverPartOfCompilation(); + Object type = cachedShape.getDynamicType(); + if (type instanceof PythonBuiltinClass pbc) { + type = pbc.getType(); + } + if (type instanceof PythonBuiltinClassType pbct) { + if (!hasObjectSetattro(pbct.getSlots())) { + return false; + } + Object descr = LookupAttributeInMRONode.findAttr(pbct, key); + return descr == PNone.NO_VALUE || isBuiltinNonDataDescr(descr); + } else if (type instanceof PythonClass klass) { + if (!hasObjectSetattro(klass.getTpSlots())) { + return false; } - return descr instanceof PythonObject pyDescr && pyDescr.getPythonClass() instanceof PythonBuiltinClassType descrType && - descrType.getSlots().tp_descr_set() == null; + Object descr = LookupAttributeInMRONode.lookupSlowPathNoSideEffects(klass, key); + return descr != null && (descr == PNone.NO_VALUE || isBuiltinNonDataDescr(descr)); } return false; } - @Idempotent - public static boolean isBuiltinWithObjectSetattro(Shape cachedShape) { - return cachedShape.getDynamicType() instanceof PythonBuiltinClassType type && type.getSlots().tp_setattro() == ObjectBuiltins.SLOTS.tp_setattro(); - } - - public static PythonManagedClass getManagedClassOrNull(Shape cachedShape) { - return cachedShape.getDynamicType() instanceof PythonManagedClass managedClass ? managedClass : null; + private static boolean isBuiltinNonDataDescr(Object descr) { + // This handles the situation where instance attribute is shadowed by a descriptor + // that's not writeable (e.g., instance attribute and a method of the same name). This is only + // valid, because we invalidate the type-stable assumption on any write and not just write of a new + // attribute and the `descr` has builtin type, whose slots cannot be modified to gain tp_descr_set + Object descrClass = GetClassNode.executeUncached(descr); + return descrClass instanceof PythonBuiltinClassType descrType && descrType.getSlots().tp_descr_set() == null; } @Idempotent @@ -2269,18 +2270,15 @@ public static boolean hasNoSlotsOrMaterializedDict(Shape cachedShape) { @ForceQuickening @Specialization(guards = { - "hasNoSlotsOrMaterializedDict(cachedShape)", "managedClass != null || isBuiltinWithObjectSetattro(cachedShape)", // - "cachedShape.check(receiver)", // - "skipDescriptorCheck || canStoreInstanceValue(inliningTarget, cachedKey, managedClass, cachedShape, getDesc, getDescSlotsNode, slotsValueProfile)"}, limit = "3") + /* static checks: */ "noDescriptor", "hasNoSlotsOrMaterializedDict(cachedShape)", // + /* dynamic checks: */ "cachedShape.check(receiver)"}, // + assumptions = "typeStableAssumption", // + limit = "3") static void doInstanceValue(TruffleString key, Object value, PythonObject receiver, - @Bind Node inliningTarget, - @Cached("key") TruffleString cachedKey, @Cached("receiver.getShape()") Shape cachedShape, - @Cached("getManagedClassOrNull(cachedShape)") PythonManagedClass managedClass, - @Cached("canSkipDescriptorCheck(cachedShape, key)") boolean skipDescriptorCheck, - @Cached LookupAttributeInMRONode.CachedKeyFastPath getDesc, - @Cached GetObjectSlotsNode getDescSlotsNode, - @Cached InlineWeakValueProfile slotsValueProfile, + @Cached("canBypassDescriptorLookupForStore(cachedShape, key)") boolean noDescriptor, + @Cached("getPythonClassOrNull(cachedShape)") PythonClass cachedPythonClass, + @Cached("getTypeStableAssumption(cachedPythonClass)") Assumption typeStableAssumption, @Cached DynamicObject.PutNode putNode) { putNode.execute(receiver, key, value); } From 95aa3f1881ed4971b15ed113b0f0b75cd98b3edd Mon Sep 17 00:00:00 2001 From: stepan Date: Fri, 11 Sep 2026 14:38:01 +0200 Subject: [PATCH 2/2] Add LazyCyclicAssumption helper class and use it where appropriate --- .../test/util/LazyCyclicAssumptionTest.java | 117 ++++++++++++++++++ .../builtins/objects/type/PythonClass.java | 26 ++-- .../objects/type/PythonManagedClass.java | 4 +- .../graal/python/runtime/PythonOptions.java | 3 - .../sequence/storage/MroSequenceStorage.java | 31 ++--- .../python/util/LazyCyclicAssumption.java | 109 ++++++++++++++++ 6 files changed, 251 insertions(+), 39 deletions(-) create mode 100644 graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/util/LazyCyclicAssumptionTest.java create mode 100644 graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/LazyCyclicAssumption.java diff --git a/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/util/LazyCyclicAssumptionTest.java b/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/util/LazyCyclicAssumptionTest.java new file mode 100644 index 0000000000..151264af94 --- /dev/null +++ b/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/util/LazyCyclicAssumptionTest.java @@ -0,0 +1,117 @@ +/* + * Copyright (c) 2026, 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 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.graal.python.test.util; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.oracle.graal.python.util.LazyCyclicAssumption; +import com.oracle.truffle.api.Assumption; + +public class LazyCyclicAssumptionTest { + @Test + public void testRenewalAndExhaustion() { + LazyCyclicAssumption cyclic = new LazyCyclicAssumption("test"); + Assumption first = cyclic.getAssumption(); + assertTrue(first.isValid()); + assertSame(first, cyclic.getAssumption()); + cyclic.invalidate(2); + assertFalse(first.isValid()); + // Invalidations without an intervening get must not exhaust the remaining budget. + cyclic.invalidate(2); + cyclic.invalidate(2); + Assumption second = cyclic.getAssumption(); + assertTrue(second.isValid()); + assertNotSame(first, second); + cyclic.invalidate(2); + assertFalse(second.isValid()); + assertSame(Assumption.NEVER_VALID, cyclic.getAssumption()); + cyclic.invalidate(Integer.MAX_VALUE); + assertSame(Assumption.NEVER_VALID, cyclic.getAssumption()); + } + + @Test + public void testInvalidationBeforeFirstGet() { + LazyCyclicAssumption cyclic = new LazyCyclicAssumption("test"); + cyclic.invalidate(2); + cyclic.invalidate(2); + Assumption first = cyclic.getAssumption(); + assertTrue(first.isValid()); + cyclic.invalidate(2); + assertFalse(first.isValid()); + Assumption second = cyclic.getAssumption(); + assertTrue(second.isValid()); + cyclic.invalidate(2); + assertFalse(second.isValid()); + assertSame(Assumption.NEVER_VALID, cyclic.getAssumption()); + } + + @Test + public void testChangingLimit() { + LazyCyclicAssumption cyclic = new LazyCyclicAssumption("test"); + cyclic.invalidate(10); + Assumption first = cyclic.getAssumption(); + cyclic.invalidate(3); + assertFalse(first.isValid()); + assertTrue(cyclic.getAssumption().isValid()); + cyclic.invalidate(2); + assertSame(Assumption.NEVER_VALID, cyclic.getAssumption()); + } + + @Test + public void testNonPositiveLimit() { + for (int limit : new int[]{0, -1, Integer.MIN_VALUE}) { + LazyCyclicAssumption cyclic = new LazyCyclicAssumption("test"); + Assumption first = cyclic.getAssumption(); + cyclic.invalidate(limit); + assertFalse(first.isValid()); + assertSame(Assumption.NEVER_VALID, cyclic.getAssumption()); + LazyCyclicAssumption unused = new LazyCyclicAssumption("unused"); + unused.invalidate(limit); + assertSame(Assumption.NEVER_VALID, unused.getAssumption()); + } + } + +} diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java index c33925c6fa..a4226ed02e 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonClass.java @@ -37,6 +37,8 @@ import com.oracle.graal.python.nodes.interop.PForeignToPTypeNode; import com.oracle.graal.python.nodes.object.GetClassNode; import com.oracle.graal.python.runtime.GilNode; +import com.oracle.graal.python.runtime.PythonContext; +import com.oracle.graal.python.util.LazyCyclicAssumption; import com.oracle.graal.python.util.SuppressFBWarnings; import com.oracle.truffle.api.Assumption; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; @@ -54,7 +56,6 @@ import com.oracle.truffle.api.profiles.InlinedBranchProfile; import com.oracle.truffle.api.source.SourceSection; import com.oracle.truffle.api.strings.TruffleString; -import com.oracle.truffle.api.utilities.CyclicAssumption; /** * Mutable class. @@ -63,7 +64,7 @@ public final class PythonClass extends PythonManagedClass { private static final int MRO_SHAPE_INVALIDATIONS_MAX = 5; - private static final int TYPE_STABLE_INVALIDATIONS_MAX = 100; + private static final int TYPE_STABLE_INVALIDATIONS_MAX = Integer.getInteger("org.graalvm.python.internal.typeStableInvalidationsMax", 10); /** * MroShape is only set if all base classes in MRO have mroShape set. @@ -73,8 +74,7 @@ public final class PythonClass extends PythonManagedClass { private byte mroShapeInvalidationsCount; /** Assumption for instance attribute fast paths, which are only used in single-context mode. */ - private final CyclicAssumption typeStableAssumption; - private byte typeStableInvalidationsCount = 0; + private final LazyCyclicAssumption typeStableAssumption; public PythonClass(Node location, PythonLanguage lang, Object typeClass, Shape classShape, TruffleString name, Object base, PythonAbstractClass[] baseClasses) { this(location, lang, typeClass, classShape, name, true, true, base, baseClasses); @@ -87,7 +87,7 @@ public PythonClass(Node location, PythonLanguage lang, Object typeClass, Shape c private PythonClass(Node location, PythonLanguage lang, Object typeClass, Shape classShape, TruffleString name, boolean invokeMro, boolean initDocAttr, Object base, PythonAbstractClass[] baseClasses) { super(location, lang, typeClass, classShape, null, name, invokeMro, initDocAttr, base, baseClasses, null); - typeStableAssumption = lang.isSingleContext() ? new CyclicAssumption("Python class stable") : null; + typeStableAssumption = lang.isSingleContext() ? new LazyCyclicAssumption(name.toJavaStringUncached()) : null; } public void setTpSlots(TpSlots tpSlots) { @@ -96,7 +96,11 @@ public void setTpSlots(TpSlots tpSlots) { } public Assumption getTypeStableAssumption() { - return typeStableAssumption == null ? Assumption.NEVER_VALID : typeStableAssumption.getAssumption(); + if (typeStableAssumption == null) { + return Assumption.NEVER_VALID; + } + assert PythonContext.get(null).ownsGil(); + return typeStableAssumption.getAssumption(); } /** @@ -110,14 +114,8 @@ public void invalidateTypeStableAssumption() { if (typeStableAssumption == null) { return; } - byte invalidationsCount = typeStableInvalidationsCount; - if (invalidationsCount < TYPE_STABLE_INVALIDATIONS_MAX) { - typeStableInvalidationsCount = (byte) (invalidationsCount + 1); - typeStableAssumption.invalidate(); - } else { - // Do not create new Assumption, just make sure that the current one is invalid - typeStableAssumption.getAssumption().invalidate(); - } + assert PythonContext.get(null).ownsGil(); + typeStableAssumption.invalidate(TYPE_STABLE_INVALIDATIONS_MAX); } public static void invalidateTypeStableAssumption(PythonAbstractClass klass) { diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java index 36dd2dd1c9..0588c2696d 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/PythonManagedClass.java @@ -103,6 +103,7 @@ protected PythonManagedClass(Node location, PythonLanguage lang, Object typeClas this.tpSlots = slots; this.methodResolutionOrder = new MroSequenceStorage(name, 0); + // following code may read and even override methodResolutionOrder if (baseClasses.length == 1 && baseClasses[0] == null) { this.baseClasses = new PythonAbstractClass[]{}; @@ -110,8 +111,7 @@ protected PythonManagedClass(Node location, PythonLanguage lang, Object typeClas unsafeSetSuperClass(baseClasses); } - // set field directly to avoid any invalidations, there is no-one that can rely on the MRO yet - methodResolutionOrder = new MroSequenceStorage(name, ComputeMroNode.doSlowPath(location, this, invokeMro)); + this.setMRO(ComputeMroNode.doSlowPath(location, this, invokeMro)); if (invokeMro) { mroInitialized = true; } diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonOptions.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonOptions.java index a79e2f1a80..f65751169a 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonOptions.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonOptions.java @@ -288,9 +288,6 @@ private PythonOptions() { @EngineOption @Option(category = OptionCategory.EXPERT, usageSyntax = "", help = "") // public static final OptionKey NodeRecursionLimit = new OptionKey<>(1); - @EngineOption @Option(category = OptionCategory.EXPERT, usageSyntax = "", help = "") // - public static final OptionKey MaxTypeInvalidationCount = new OptionKey<>(3); - @Option(category = OptionCategory.EXPERT, usageSyntax = "true|false", help = "Force to automatically import site.py module.", stability = OptionStability.STABLE) // public static final OptionKey ForceImportSite = new OptionKey<>(false); diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java index a7005102a5..6c316ace5c 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java @@ -43,9 +43,9 @@ import java.util.HashMap; import java.util.Map; -import com.oracle.graal.python.PythonLanguage; import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass; -import com.oracle.graal.python.runtime.PythonOptions; +import com.oracle.graal.python.runtime.PythonContext; +import com.oracle.graal.python.util.LazyCyclicAssumption; import com.oracle.graal.python.util.PythonUtils; import com.oracle.truffle.api.Assumption; import com.oracle.truffle.api.CompilerAsserts; @@ -59,12 +59,13 @@ public final class MroSequenceStorage extends ArrayBasedSequenceStorage { + private static final int LOOKUP_STABLE_INVALIDATIONS_MAX = Integer.getInteger("org.graalvm.python.internal.lookupStableInvalidationsMax", 3); + private final TruffleString className; /** * This assumption will be invalidated whenever the mro changes. */ - private Assumption lookupStableAssumption; - private int lookupStableAssumptionInvalidations; + private final LazyCyclicAssumption lookupStableAssumption; /** * These assumptions will be invalidated whenever the value of the given slot changes. All @@ -125,7 +126,7 @@ public MroSequenceStorage(TruffleString className, PythonAbstractClass[] element this.values = elements; this.capacity = elements.length; this.length = elements.length; - this.lookupStableAssumption = createLookupStableAssumption(); + this.lookupStableAssumption = new LazyCyclicAssumption(className.toJavaStringUncached()); this.attributesInMROFinalAssumptions = new HashMap<>(); } @@ -135,7 +136,7 @@ public MroSequenceStorage(TruffleString className, int capacity) { this.values = new PythonAbstractClass[capacity]; this.capacity = capacity; this.length = 0; - this.lookupStableAssumption = createLookupStableAssumption(); + this.lookupStableAssumption = new LazyCyclicAssumption(className.toJavaStringUncached()); this.attributesInMROFinalAssumptions = new HashMap<>(); } @@ -188,7 +189,8 @@ public StorageType getElementType() { } public Assumption getLookupStableAssumption() { - return lookupStableAssumption; + assert PythonContext.get(null).ownsGil(); + return lookupStableAssumption.getAssumption(); } public FinalAttributeAssumptionPair getFinalAttributeAssumption(TruffleString name) { @@ -218,24 +220,13 @@ public void invalidateFinalAttributeAssumption(TruffleString name) { public void lookupChanged() { CompilerAsserts.neverPartOfCompilation(); + assert PythonContext.get(null).ownsGil(); if (attributesInMROFinalAssumptions != null) { for (FinalAttributeAssumptionPair assumptionPair : attributesInMROFinalAssumptions.values()) { assumptionPair.invalidate(); } } - if (lookupStableAssumption != Assumption.NEVER_VALID) { - lookupStableAssumption.invalidate(); - if (lookupStableAssumptionInvalidations < PythonLanguage.get(null).getEngineOption(PythonOptions.MaxTypeInvalidationCount)) { - lookupStableAssumption = createLookupStableAssumption(); - lookupStableAssumptionInvalidations++; - } else { - lookupStableAssumption = Assumption.NEVER_VALID; - } - } - } - - private Assumption createLookupStableAssumption() { - return Truffle.getRuntime().createAssumption(className.toJavaStringUncached()); + lookupStableAssumption.invalidate(LOOKUP_STABLE_INVALIDATIONS_MAX); } public NativeSequenceStorage getNativeMirror() { diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/LazyCyclicAssumption.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/LazyCyclicAssumption.java new file mode 100644 index 0000000000..8ee4fae643 --- /dev/null +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/LazyCyclicAssumption.java @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2026, 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 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.graal.python.util; + +import com.oracle.graal.python.PythonLanguage; +import com.oracle.truffle.api.Assumption; +import com.oracle.truffle.api.CompilerAsserts; +import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.Truffle; +import com.oracle.truffle.api.TruffleLogger; +import com.oracle.truffle.api.utilities.CyclicAssumption; + +/** + * A lazy {@link CyclicAssumption} with a bounded number of invalidations. Obtain the assumption + * before reading the value to cache, and invalidate after changing that value. + *

+ * Assumptions are allocated only by {@link #getAssumption()}. Once the limit supplied to + * {@link #invalidate(int)} is reached, this helper permanently returns {@link Assumption#NEVER_VALID}. + * The caller is responsible to suply constant limit to {@link #invalidate(int)}. + *

+ * The caller must protect all accesses with the same lock. This helper deliberately performs no + * synchronization of its own. + */ +public final class LazyCyclicAssumption { + private static final TruffleLogger LOGGER = PythonLanguage.getLogger(LazyCyclicAssumption.class); + + private final String name; + private Assumption assumption; + private int invalidationCount; + + public LazyCyclicAssumption(String name) { + this.name = name; + } + + public Assumption getAssumption() { + CompilerAsserts.neverPartOfCompilation(); + Assumption current = assumption; + if (current != null) { + return current; + } + Assumption created = Truffle.getRuntime().createAssumption(name); + assumption = created; + return created; + } + + /** + * Invalidates the current assumption, if any. After {@code limit} assumptions have been removed, + * assumption creation is permanently disabled. Calls that find no assumption do not advance the count. + *

+ * No replacement assumption is allocated until the next {@link #getAssumption()}. + */ + @TruffleBoundary + public void invalidate(int limit) { + Assumption current = assumption; + if (current == Assumption.NEVER_VALID) { + return; + } + if (current != null) { + current.invalidate(); + if (invalidationCount < Integer.MAX_VALUE) { + invalidationCount++; + } + } + if (invalidationCount >= limit) { + LOGGER.fine(() -> "Assumption '" + name + "' reached invalidation limit of " + limit); + assumption = Assumption.NEVER_VALID; + } else { + assumption = null; + } + } +}