Simplify attribute writes

Author: msimacek

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/HashingStorageNodes.java

@@ -444,6 +444,10 @@ static HashingStorage foreign(Node inliningTarget, ForeignHashingStorage self, O
             return self;
         }
 
+        public static HashingStorageSetItem getUncached() {
+            return HashingStorageSetItemNodeGen.getUncached();
+        }
+
         @GenerateUncached
         @GenerateInline
         @GenerateCached(false)

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

@@ -282,12 +282,6 @@ public boolean hasMroShapeSubTypes() {
         return mroShapeSubTypes != null;
     }
 
-    @Override
-    public boolean canSkipOnAttributeUpdate(TruffleString key, @SuppressWarnings("unused") Object newValue, TruffleString.CodePointLengthNode codePointLengthNode,
-                    TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
-        return super.canSkipOnAttributeUpdate(key, newValue, codePointLengthNode, codePointAtIndexNode) && mroShapeSubTypes == null;
-    }
-
     @TruffleBoundary
     @Override
     public void onAttributeUpdate(TruffleString key, Object newValue) {

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

@@ -207,16 +207,6 @@ public void setAttribute(TruffleString key, Object value) {
         onAttributeUpdate(key, value);
     }
 
-    /**
-     * Fast-path check designed for PE code.
-     */
-    public boolean canSkipOnAttributeUpdate(TruffleString key, @SuppressWarnings("unused") Object value, TruffleString.CodePointLengthNode codePointLengthNode,
-                    TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
-        // TODO subclasses
-        return !methodResolutionOrder.hasAttributeInMROFinalAssumptions() &&
-                        !TpSlots.canBeSpecialMethod(key, codePointLengthNode, codePointAtIndexNode);
-    }
-
     @TruffleBoundary
     public void onAttributeUpdate(TruffleString key, Object value) {
         for (PythonAbstractClass subclass : GetSubclassesAsArrayNode.executeUncached(this)) {

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

@@ -60,13 +60,13 @@
 import com.oracle.graal.python.builtins.objects.type.TpSlots;
 import com.oracle.graal.python.builtins.objects.type.TypeFlags;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.IsTypeNode;
-import com.oracle.graal.python.lib.PyObjectReprAsTruffleStringNode;
 import com.oracle.graal.python.nodes.ErrorMessages;
 import com.oracle.graal.python.nodes.PNodeWithContext;
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.object.GetDictIfExistsNode;
 import com.oracle.graal.python.runtime.PythonContext;
 import com.oracle.truffle.api.CompilerAsserts;
+import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Bind;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Exclusive;
@@ -95,7 +95,7 @@ static boolean isAttrWritable(PythonObject self) {
         return (self.getShape().getFlags() & PythonObject.HAS_SLOTS_BUT_NO_DICT_FLAG) == 0;
     }
 
-    static boolean writeToDynamicStorageNoTypeGuard(Object obj, GetDictIfExistsNode getDict) {
+    static boolean writeToDynamicStorageNoTypeGuard(PythonObject obj, GetDictIfExistsNode getDict) {
         return getDict.execute(obj) == null && !PythonManagedClass.isInstance(obj);
     }
 
@@ -112,49 +112,48 @@ static boolean writeToDynamicStorageNoType(PythonObject object, TruffleString ke
         return true;
     }
 
-    // Specializations for no dict & PythonManagedClass -> requires calling onAttributeUpdate
-    @Specialization(guards = {"isAttrWritable(klass)", "getDict.execute(klass) == null"})
-    boolean writeToDynamicStorageBuiltinType(PythonBuiltinClass klass, TruffleString key, Object value,
-                    @Bind Node inliningTarget,
-                    @SuppressWarnings("unused") @Shared("getDict") @Cached GetDictIfExistsNode getDict,
-                    @Shared("callAttrUpdate") @Cached InlinedBranchProfile callAttrUpdate,
-                    @Shared("dylib") @CachedLibrary(limit = "getAttributeAccessInlineCacheMaxDepth()") DynamicObjectLibrary dylib,
-                    @Shared("cpLen") @Cached TruffleString.CodePointLengthNode codePointLengthNode,
-                    @Shared("cpAtIndex") @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
-        if (PythonContext.get(this).isInitialized()) {
+    @Specialization(guards = "isPythonBuiltinClass(klassOrType) || isPythonBuiltinClassType(klassOrType)")
+    @TruffleBoundary
+    boolean writeToDynamicStorageBuiltinType(Object klassOrType, TruffleString key, Object value) {
+        PythonContext context = PythonContext.get(this);
+        PythonBuiltinClass klass;
+        if (klassOrType instanceof PythonBuiltinClass pbc) {
+            klass = pbc;
+        } else {
+            klass = context.lookupType((PythonBuiltinClassType) klassOrType);
+        }
+        if (context.isInitialized() || value == PNone.NO_VALUE) {
             throw PRaiseNode.raiseStatic(this, TypeError, ErrorMessages.CANT_SET_ATTRIBUTE_R_OF_IMMUTABLE_TYPE_N, key, klass);
         } else {
-            return writeToDynamicStorageManagedClass(klass, key, value, inliningTarget, callAttrUpdate, dylib, codePointLengthNode, codePointAtIndexNode);
+            PDict dict = GetDictIfExistsNode.getUncached().execute(klass);
+            if (dict == null) {
+                return writeToDynamicStorageManagedClass(klass, key, value, DynamicObjectLibrary.getUncached());
+            } else {
+                return writeToDictManagedClass(klass, dict, key, value, null, InlinedBranchProfile.getUncached(), HashingStorageSetItem.getUncached());
+            }
         }
     }
 
     @Specialization(guards = {"isAttrWritable(klass)", "getDict.execute(klass) == null"})
     static boolean writeToDynamicStoragePythonClass(PythonClass klass, TruffleString key, Object value,
                     @Bind Node inliningTarget,
                     @SuppressWarnings("unused") @Shared("getDict") @Cached GetDictIfExistsNode getDict,
-                    @Exclusive @Cached InlinedBranchProfile callAttrUpdate,
                     @Exclusive @Cached InlinedBranchProfile updateFlags,
-                    @Shared("dylib") @CachedLibrary(limit = "getAttributeAccessInlineCacheMaxDepth()") DynamicObjectLibrary dylib,
-                    @Shared("cpLen") @Cached TruffleString.CodePointLengthNode codePointLengthNode,
-                    @Shared("cpAtIndex") @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
+                    @CachedLibrary(limit = "getAttributeAccessInlineCacheMaxDepth()") DynamicObjectLibrary dylib) {
         if (value == PNone.NO_VALUE) {
             updateFlags.enter(inliningTarget);
             dylib.setShapeFlags(klass, dylib.getShapeFlags(klass) | HAS_NO_VALUE_PROPERTIES);
         }
-        return writeToDynamicStorageManagedClass(klass, key, value, inliningTarget, callAttrUpdate, dylib, codePointLengthNode, codePointAtIndexNode);
+        return writeToDynamicStorageManagedClass(klass, key, value, dylib);
     }
 
-    private static boolean writeToDynamicStorageManagedClass(PythonManagedClass klass, TruffleString key, Object value, Node inliningTarget,
-                    InlinedBranchProfile callAttrUpdate, DynamicObjectLibrary dylib, TruffleString.CodePointLengthNode codePointLengthNode, TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
+    private static boolean writeToDynamicStorageManagedClass(PythonManagedClass klass, TruffleString key, Object value, DynamicObjectLibrary dylib) {
         CompilerAsserts.partialEvaluationConstant(klass.getClass());
         try {
             dylib.put(klass, key, value);
             return true;
         } finally {
-            if (!klass.canSkipOnAttributeUpdate(key, value, codePointLengthNode, codePointAtIndexNode)) {
-                callAttrUpdate.enter(inliningTarget);
-                klass.onAttributeUpdate(key, value);
-            }
+            klass.onAttributeUpdate(key, value);
         }
     }
 
@@ -169,35 +168,14 @@ static boolean writeToDictNoType(@SuppressWarnings("unused") PythonObject object
         return writeToDict(dict, key, value, inliningTarget, updateStorage, setHashingStorageItem);
     }
 
-    // write to the dict & PythonManagedClass -> requires calling onAttributeUpdate
-    @Specialization(guards = {"dict != null", "!isNoValue(value)"})
-    boolean writeToDictBuiltinType(PythonBuiltinClass klass, TruffleString key, Object value,
-                    @Bind Node inliningTarget,
-                    @SuppressWarnings("unused") @Shared("getDict") @Cached GetDictIfExistsNode getDict,
-                    @Bind("getDict.execute(klass)") PDict dict,
-                    @Shared("callAttrUpdate") @Cached InlinedBranchProfile callAttrUpdate,
-                    @Shared("updateStorage") @Cached InlinedBranchProfile updateStorage,
-                    @Shared("setHashingStorageItem") @Cached HashingStorageSetItem setHashingStorageItem,
-                    @Shared("cpLen") @Cached TruffleString.CodePointLengthNode codePointLengthNode,
-                    @Shared("cpAtIndex") @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
-        if (PythonContext.get(this).isInitialized()) {
-            throw PRaiseNode.raiseStatic(this, TypeError, ErrorMessages.CANT_SET_ATTRIBUTE_R_OF_IMMUTABLE_TYPE_N, PyObjectReprAsTruffleStringNode.executeUncached(key), klass);
-        } else {
-            return writeToDictManagedClass(klass, dict, key, value, inliningTarget, callAttrUpdate, updateStorage, setHashingStorageItem, codePointLengthNode, codePointAtIndexNode);
-        }
-    }
-
     @Specialization(guards = {"dict != null", "!isNoValue(value)"})
     static boolean writeToDictClass(PythonClass klass, TruffleString key, Object value,
                     @Bind Node inliningTarget,
                     @SuppressWarnings("unused") @Shared("getDict") @Cached GetDictIfExistsNode getDict,
                     @Bind("getDict.execute(klass)") PDict dict,
-                    @Shared("callAttrUpdate") @Cached InlinedBranchProfile callAttrUpdate,
                     @Shared("updateStorage") @Cached InlinedBranchProfile updateStorage,
-                    @Shared("setHashingStorageItem") @Cached HashingStorageSetItem setHashingStorageItem,
-                    @Shared("cpLen") @Cached TruffleString.CodePointLengthNode codePointLengthNode,
-                    @Shared("cpAtIndex") @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
-        return writeToDictManagedClass(klass, dict, key, value, inliningTarget, callAttrUpdate, updateStorage, setHashingStorageItem, codePointLengthNode, codePointAtIndexNode);
+                    @Shared("setHashingStorageItem") @Cached HashingStorageSetItem setHashingStorageItem) {
+        return writeToDictManagedClass(klass, dict, key, value, inliningTarget, updateStorage, setHashingStorageItem);
     }
 
     // @Exclusive for truffle-interpreted-performance
@@ -206,42 +184,24 @@ static boolean deleteFromPythonObject(PythonObject obj, TruffleString key, Objec
                     @Bind Node inliningTarget,
                     @SuppressWarnings("unused") @Shared("getDict") @Cached GetDictIfExistsNode getDict,
                     @Bind("getDict.execute(obj)") PDict dict,
-                    @Exclusive @Cached InlinedBranchProfile callAttrUpdate,
-                    @Cached HashingStorageNodes.HashingStorageDelItem hashingStorageDelItem,
-                    @Shared("cpLen") @Cached TruffleString.CodePointLengthNode codePointLengthNode,
-                    @Shared("cpAtIndex") @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
+                    @Cached HashingStorageNodes.HashingStorageDelItem hashingStorageDelItem) {
         try {
             HashingStorage dictStorage = dict.getDictStorage();
             return hashingStorageDelItem.execute(inliningTarget, dictStorage, key, dict);
         } finally {
             if (obj instanceof PythonManagedClass klass) {
-                if (!klass.canSkipOnAttributeUpdate(key, value, codePointLengthNode, codePointAtIndexNode)) {
-                    callAttrUpdate.enter(inliningTarget);
-                    klass.onAttributeUpdate(key, value);
-                }
+                klass.onAttributeUpdate(key, value);
             }
         }
     }
 
-    @Specialization(guards = {"dict != null", "isNoValue(value)"})
-    static boolean deleteFromPythonBuiltinClass(PythonBuiltinClass klass, TruffleString key, Object value,
-                    @Bind Node inliningTarget,
-                    @SuppressWarnings("unused") @Shared("getDict") @Cached GetDictIfExistsNode getDict,
-                    @Bind("getDict.execute(klass)") PDict dict) {
-        throw PRaiseNode.raiseStatic(inliningTarget, TypeError, ErrorMessages.CANT_SET_ATTRIBUTE_R_OF_IMMUTABLE_TYPE_N, key, klass);
-    }
-
     private static boolean writeToDictManagedClass(PythonManagedClass klass, PDict dict, TruffleString key, Object value, Node inliningTarget,
-                    InlinedBranchProfile callAttrUpdate, InlinedBranchProfile updateStorage, HashingStorageSetItem setHashingStorageItem, TruffleString.CodePointLengthNode codePointLengthNode,
-                    TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
+                    InlinedBranchProfile updateStorage, HashingStorageSetItem setHashingStorageItem) {
         CompilerAsserts.partialEvaluationConstant(klass.getClass());
         try {
             return writeToDict(dict, key, value, inliningTarget, updateStorage, setHashingStorageItem);
         } finally {
-            if (!klass.canSkipOnAttributeUpdate(key, value, codePointLengthNode, codePointAtIndexNode)) {
-                callAttrUpdate.enter(inliningTarget);
-                klass.onAttributeUpdate(key, value);
-            }
+            klass.onAttributeUpdate(key, value);
         }
     }
 
@@ -261,12 +221,6 @@ static boolean writeToDict(PDict dict, TruffleString key, Object value,
         return true;
     }
 
-    @Specialization
-    static boolean doPBCT(PythonBuiltinClassType object, TruffleString key, Object value,
-                    @Cached WriteAttributeToObjectNode recursive) {
-        return recursive.execute(PythonContext.get(recursive).lookupType(object), key, value);
-    }
-
     private static void checkNativeImmutable(Node inliningTarget, PythonAbstractNativeObject object, TruffleString key,
                     CStructAccess.ReadI64Node getNativeFlags,
                     PRaiseNode raiseNode) {
@@ -288,8 +242,8 @@ static boolean writeNativeObjectOrClass(PythonAbstractNativeObject object, Truff
                     @Exclusive @Cached InlinedBranchProfile canBeSpecialSlot,
                     @Cached IsTypeNode isTypeNode,
                     @Exclusive @Cached PRaiseNode raiseNode,
-                    @Shared("cpLen") @Cached TruffleString.CodePointLengthNode codePointLengthNode,
-                    @Shared("cpAtIndex") @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
+                    @Cached TruffleString.CodePointLengthNode codePointLengthNode,
+                    @Cached TruffleString.CodePointAtIndexNode codePointAtIndexNode) {
         boolean isType = isTypeProfile.profile(inliningTarget, isTypeNode.execute(inliningTarget, object));
         try {
             Object dict;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java

@@ -213,10 +213,6 @@ public void lookupChanged() {
         lookupStableAssumption.invalidate();
     }
 
-    public boolean hasAttributeInMROFinalAssumptions() {
-        return attributesInMROFinalAssumptions != null;
-    }
-
     public NativeSequenceStorage getNativeMirror() {
         return nativeMirror;
     }