Use Object[] for __slots__ instead of HiddenAttrs

Author: otethal

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_indexed_slots.py

@@ -0,0 +1,132 @@
+# Copyright (c) 2024, 2024, 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.
+
+from . import CPyExtTestCase, CPyExtHeapType
+__dir__ = __file__.rpartition("/")[0]
+
+
+class BaseWithSlots:
+    __slots__ = ('a', 'b')
+
+class AnotherBaseWithSlots:
+    __slots__ = ('b',)
+
+class EmptyBase:
+    pass
+
+# This is used to increase tp_basicsize in CPyExtHeapType which needs to be grater that the basic size of the
+# base class. We do not use the C struct in any way - in fact in CPython, the `space` member declared here
+# overlaps with __slots__ of the base class.
+def cmembers(i):
+    return f"long space[{i}];"
+
+class TestIndexedSlots(CPyExtTestCase):
+    def test_slots_in_base(self):
+        N1 = CPyExtHeapType('Na1', bases=(BaseWithSlots,), cmembers=cmembers(4))
+        N2 = CPyExtHeapType('Na2', bases=(N1,), cmembers=cmembers(8))
+
+        class M1(EmptyBase, N2):
+            pass
+
+        class M2(N2, EmptyBase):
+            pass
+
+        for C in (N1, N2, M1, M2):
+            x = C()
+            x.a, x.b = 12, 13
+            self.assertEqual((12, 13), (x.a, x.b))
+
+    def test_slots_in_subclass(self):
+        N1 = CPyExtHeapType('Nb1', bases=(EmptyBase,), cmembers=cmembers(4))
+
+        class M1(N1):
+            __slots__ = ('a', 'b')
+
+        class M2(N1):
+            __slots__ = ('a', 'b')
+
+        class M3(M1):
+            __slots__ = ('c',)
+
+        for C in (M1, M2):
+            x = C()
+            x.a, x.b = 12, 13
+            self.assertEqual((12, 13), (x.a, x.b))
+
+        x = M3()
+        x.a, x.b, x.c = 22, 23, 24
+        self.assertEqual((22, 23, 24), (x.a, x.b, x.c))
+
+
+    def test_slots_in_base_and_subclass(self):
+        N1 = CPyExtHeapType('Nd1', bases=(BaseWithSlots,), cmembers=cmembers(4))
+
+        class M1(N1):
+            __slots__ = ('c', 'd')
+
+        class M2(N1):
+            __slots__ = ('c', 'd', 'a')
+
+        class M3(M1):
+            __slots__ = ('b', 'a')
+
+        for C in (M1, M2, M3):
+            x = C()
+            x.a, x.b, x.c, x.d = 12, 13, 14, 15
+            self.assertEqual((12, 13, 14, 15), (x.a, x.b, x.c, x.d))
+
+    def test_multi_bases(self):
+        N1 = CPyExtHeapType('Ne1', bases=(BaseWithSlots, EmptyBase), cmembers=cmembers(6))
+        N2 = CPyExtHeapType('Ne2', bases=(EmptyBase, BaseWithSlots), cmembers=cmembers(6))
+
+        class M1(N1):
+            pass
+
+        class M2(N1):
+            __slots__ = ('b',)
+
+        for C in (N1, N2, M1, M2):
+            x = C()
+            x.a, x.b = 12, 13
+            self.assertEqual((12, 13), (x.a, x.b))
+
+    def test_layout_conflict(self):
+        N1 = CPyExtHeapType('Nf1', bases=(BaseWithSlots,), cmembers=cmembers(4))
+        self.assertRaises(TypeError, CPyExtHeapType, 'Nf2', bases=(AnotherBaseWithSlots, BaseWithSlots))
+        self.assertRaises(TypeError, CPyExtHeapType, 'Nf3', bases=(AnotherBaseWithSlots, N1))

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

@@ -74,7 +74,6 @@
 import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
 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.TypeBuiltins;
 import com.oracle.graal.python.builtins.objects.type.slots.TpSlot;
 import com.oracle.graal.python.compiler.CodeUnit;
 import com.oracle.graal.python.compiler.CompilationUnit;
@@ -375,8 +374,6 @@ public boolean isSingleContext() {
     /** For fast access to the PythonThreadState object by the owning thread. */
     private final ContextThreadLocal<PythonThreadState> threadState = locals.createContextThreadLocal(PythonContext.PythonThreadState::new);
 
-    public final ConcurrentHashMap<String, HiddenAttr> typeHiddenAttrs = new ConcurrentHashMap<>(TypeBuiltins.INITIAL_HIDDEN_TYPE_ATTRS);
-
     private final MroShape mroShapeRoot = MroShape.createRoot();
 
     /**

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/PythonAbstractObject.java

@@ -198,6 +198,8 @@ public abstract class PythonAbstractObject extends DynamicObject implements Truf
 
     protected static final Shape ABSTRACT_SHAPE = Shape.newBuilder().build();
 
+    private Object[] indexedSlots;
+
     protected PythonAbstractObject(Shape shape) {
         super(shape);
     }
@@ -223,6 +225,14 @@ public final void clearNativeWrapper() {
         nativeWrapper = null;
     }
 
+    public Object[] getIndexedSlots() {
+        return indexedSlots;
+    }
+
+    public void setIndexedSlots(Object[] indexedSlots) {
+        this.indexedSlots = indexedSlots;
+    }
+
     @ExportMessage
     public void writeMember(String key, Object value,
                     @Bind("$node") Node inliningTarget,

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CExtNodes.java

@@ -215,6 +215,7 @@ public abstract class CExtNodes {
 
     private static final String J_UNICODE = "unicode";
     private static final String J_SUBTYPE_NEW = "_subtype_new";
+    private static final long SIZEOF_PY_OBJECT_PTR = Long.BYTES;
 
     /**
      * For some builtin classes, the CPython approach to creating a subclass instance is to just
@@ -939,6 +940,8 @@ public static long lookupNativeI64MemberInMRO(Object cls, CFields nativeMemberNa
         MroSequenceStorage mroStorage = GetMroStorageNode.executeUncached(cls);
         int n = mroStorage.length();
 
+        boolean isBasicsizeOrWeaklistoffset = nativeMemberName == CFields.PyTypeObject__tp_basicsize || nativeMemberName == CFields.PyTypeObject__tp_weaklistoffset;
+        long indexedSlotsSize = isBasicsizeOrWeaklistoffset && cls instanceof PythonManagedClass pmc ? pmc.getIndexedSlotCount() * SIZEOF_PY_OBJECT_PTR : 0;
         for (int i = 0; i < n; i++) {
             PythonAbstractClass mroCls = (PythonAbstractClass) SequenceStorageNodes.GetItemDynamicNode.executeUncached(mroStorage, i);
 
@@ -950,15 +953,15 @@ public static long lookupNativeI64MemberInMRO(Object cls, CFields nativeMemberNa
                     attr = ReadAttributeFromObjectNode.getUncachedForceType().execute(mroCls, CompilerDirectives.castExact(managedMemberName, TruffleString.class));
                 }
                 if (attr != NO_VALUE) {
-                    return PyNumberAsSizeNode.executeExactUncached(attr);
+                    return indexedSlotsSize + PyNumberAsSizeNode.executeExactUncached(attr);
                 }
             } else {
                 assert PGuards.isNativeClass(mroCls) : "invalid class inheritance structure; expected native class";
-                return CStructAccess.ReadI64Node.getUncached().readFromObj((PythonNativeClass) mroCls, nativeMemberName);
+                return indexedSlotsSize + CStructAccess.ReadI64Node.getUncached().readFromObj((PythonNativeClass) mroCls, nativeMemberName);
             }
         }
         // return the value from PyBaseObject - assumed to be 0 for vectorcall_offset
-        return nativeMemberName == CFields.PyTypeObject__tp_basicsize || nativeMemberName == CFields.PyTypeObject__tp_weaklistoffset ? CStructs.PyObject.size() : 0L;
+        return isBasicsizeOrWeaklistoffset ? indexedSlotsSize + CStructs.PyObject.size() : 0L;
     }
 
     /**
@@ -986,6 +989,8 @@ static long doSingleContext(Object cls, CFields nativeMember, HiddenAttr managed
             CompilerAsserts.partialEvaluationConstant(builtinCallback);
 
             Object current = cls;
+            boolean isBasicsizeOrWeaklistoffset = nativeMember == CFields.PyTypeObject__tp_basicsize || nativeMember == CFields.PyTypeObject__tp_weaklistoffset;
+            long indexedSlotsSize = isBasicsizeOrWeaklistoffset && cls instanceof PythonManagedClass pmc ? pmc.getIndexedSlotCount() * SIZEOF_PY_OBJECT_PTR : 0;
             do {
                 if (current instanceof PythonBuiltinClassType pbct) {
                     current = PythonContext.get(inliningTarget).lookupType(pbct);
@@ -995,16 +1000,16 @@ static long doSingleContext(Object cls, CFields nativeMember, HiddenAttr managed
                 } else if (PGuards.isManagedClass(current)) {
                     Object attr = readAttrNode.execute(inliningTarget, (PythonObject) current, managedMemberName, null);
                     if (attr != null) {
-                        return asSizeNode.executeExact(null, inliningTarget, attr);
+                        return indexedSlotsSize + asSizeNode.executeExact(null, inliningTarget, attr);
                     }
                 } else {
                     assert PGuards.isNativeClass(current) : "invalid class inheritance structure; expected native class";
-                    return getTypeMemberNode.readFromObj((PythonNativeClass) current, nativeMember);
+                    return indexedSlotsSize + getTypeMemberNode.readFromObj((PythonNativeClass) current, nativeMember);
                 }
                 current = getBaseClassNode.execute(inliningTarget, current);
             } while (current != null);
             // return the value from PyBaseObject - assumed to be 0 for vectorcall_offset
-            return nativeMember == CFields.PyTypeObject__tp_basicsize || nativeMember == CFields.PyTypeObject__tp_weaklistoffset ? CStructs.PyObject.size() : 0L;
+            return isBasicsizeOrWeaklistoffset ? indexedSlotsSize + CStructs.PyObject.size() : 0L;
         }
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/getsetdescriptor/DescriptorBuiltins.java

@@ -56,9 +56,9 @@
 import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
 import com.oracle.graal.python.builtins.objects.str.StringUtils.SimpleTruffleStringFormatNode;
+import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetIndexedSlotsCountNode;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetNameNode;
 import com.oracle.graal.python.nodes.ErrorMessages;
-import com.oracle.graal.python.nodes.HiddenAttr;
 import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.attributes.GetAttributeNode.GetFixedAttributeNode;
 import com.oracle.graal.python.nodes.call.special.CallBinaryMethodNode;
@@ -106,10 +106,10 @@ static TruffleString doGetSetDescriptor(VirtualFrame frame, GetSetDescriptor sel
         }
 
         @Specialization
-        static TruffleString doHiddenAttrDescriptor(VirtualFrame frame, HiddenAttrDescriptor self,
+        static TruffleString doIndexedSlotDescriptor(VirtualFrame frame, IndexedSlotDescriptor self,
                         @Shared @Cached("create(T___QUALNAME__)") GetFixedAttributeNode readQualNameNode,
                         @Shared("formatter") @Cached SimpleTruffleStringFormatNode simpleTruffleStringFormatNode) {
-            return simpleTruffleStringFormatNode.format("%s.%s", toStr(readQualNameNode.executeObject(frame, self.getType())), self.getAttr().getName());
+            return simpleTruffleStringFormatNode.format("%s.%s", toStr(readQualNameNode.executeObject(frame, self.getType())), self.getName());
         }
 
         @TruffleBoundary
@@ -128,9 +128,8 @@ static TruffleString doGetSetDescriptor(GetSetDescriptor self) {
         }
 
         @Specialization
-        static TruffleString doHiddenAttrDescriptor(HiddenAttrDescriptor self,
-                        @Cached TruffleString.FromJavaStringNode fromJavaStringNode) {
-            return fromJavaStringNode.execute(self.getAttr().getName(), TS_ENCODING);
+        static TruffleString doIndexedSlotDescriptor(IndexedSlotDescriptor self) {
+            return self.getName();
         }
     }
 
@@ -172,15 +171,16 @@ Object doGetSetDescriptor(VirtualFrame frame, GetSetDescriptor descr, Object obj
         }
 
         @Specialization
-        Object doHiddenAttrDescriptor(HiddenAttrDescriptor descr, PythonAbstractObject obj,
+        Object doIndexedSlotDescriptor(IndexedSlotDescriptor descr, PythonAbstractObject obj,
                         @Bind("this") Node inliningTarget,
                         @Exclusive @Cached PRaiseNode.Lazy raiseNode,
-                        @Cached HiddenAttr.ReadNode readNode) {
-            Object val = readNode.execute(inliningTarget, obj, descr.getAttr(), PNone.NO_VALUE);
-            if (val != PNone.NO_VALUE) {
+                        @Cached GetOrCreateIndexedSlots getSlotsNode) {
+            Object[] slots = getSlotsNode.execute(inliningTarget, obj);
+            Object val = slots[descr.getIndex()];
+            if (val != null) {
                 return val;
             }
-            throw raiseNode.get(inliningTarget).raise(AttributeError, ErrorMessages.OBJ_N_HAS_NO_ATTR_S, descr.getType(), descr.getAttr().getName());
+            throw raiseNode.get(inliningTarget).raise(AttributeError, ErrorMessages.OBJ_N_HAS_NO_ATTR_S, descr.getType(), descr.getName());
         }
     }
 
@@ -202,10 +202,10 @@ Object doGetSetDescriptor(VirtualFrame frame, GetSetDescriptor descr, Object obj
         }
 
         @Specialization
-        static Object doHiddenAttrDescriptor(HiddenAttrDescriptor descr, PythonAbstractObject obj, Object value,
+        static Object doIndexedSlotDescriptor(IndexedSlotDescriptor descr, PythonAbstractObject obj, Object value,
                         @Bind("this") Node inliningTarget,
-                        @Cached HiddenAttr.WriteNode writeNode) {
-            writeNode.execute(inliningTarget, obj, descr.getAttr(), value);
+                        @Cached GetOrCreateIndexedSlots getSlotsNode) {
+            getSlotsNode.execute(inliningTarget, obj)[descr.getIndex()] = value;
             return true;
         }
     }
@@ -239,18 +239,41 @@ Object doGetSetDescriptor(VirtualFrame frame, GetSetDescriptor descr, Object obj
         }
 
         @Specialization
-        Object doHiddenAttrDescriptor(HiddenAttrDescriptor descr, PythonAbstractObject obj,
+        Object doIndexedSlotDescriptor(IndexedSlotDescriptor descr, PythonAbstractObject obj,
                         @Bind("this") Node inliningTarget,
                         @Exclusive @Cached PRaiseNode.Lazy raiseNode,
-                        @Cached HiddenAttr.WriteNode writeNode,
-                        @Cached HiddenAttr.ReadNode readNode,
+                        @Cached GetOrCreateIndexedSlots getSlotsNode,
                         @Cached InlinedConditionProfile profile) {
             // PyMember_SetOne - Check if the attribute is set.
-            if (profile.profile(inliningTarget, readNode.execute(inliningTarget, obj, descr.getAttr(), PNone.NO_VALUE) != PNone.NO_VALUE)) {
-                writeNode.execute(inliningTarget, obj, descr.getAttr(), PNone.NO_VALUE);
+            Object[] slots = getSlotsNode.execute(inliningTarget, obj);
+            if (profile.profile(inliningTarget, slots[descr.getIndex()] != null)) {
+                slots[descr.getIndex()] = null;
                 return PNone.NONE;
             }
-            throw raiseNode.get(inliningTarget).raise(PythonBuiltinClassType.AttributeError, ErrorMessages.S, descr.getAttr().getName());
+            throw raiseNode.get(inliningTarget).raise(PythonBuiltinClassType.AttributeError, ErrorMessages.S, descr.getName());
+        }
+    }
+
+    @GenerateInline
+    @GenerateCached(false)
+    @GenerateUncached
+    abstract static class GetOrCreateIndexedSlots extends Node {
+        abstract Object[] execute(Node inliningTarget, PythonAbstractObject object);
+
+        @Specialization(guards = "object.getIndexedSlots() != null")
+        static Object[] doGet(PythonAbstractObject object) {
+            return object.getIndexedSlots();
+        }
+
+        @Specialization(guards = "object.getIndexedSlots() == null")
+        static Object[] doCreate(Node inliningTarget, PythonAbstractObject object,
+                        @Cached GetIndexedSlotsCountNode getIndexedSlotsCountNode,
+                        @Cached GetClassNode getClassNode) {
+            Object cls = getClassNode.execute(inliningTarget, object);
+            int slotCount = getIndexedSlotsCountNode.execute(inliningTarget, cls);
+            Object[] slots = new Object[slotCount];
+            object.setIndexedSlots(slots);
+            return slots;
         }
     }
 }