Remove the ObjectHashMap.putUncachedWithJavaEq hack, because tp_richcompare slots for builtins are now initialized before the rest of the runtime

steve-s · steve-s · commit 72dfb194cd5f · 2025-04-30T09:28:45.000+02:00
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/ErrnoModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/ErrnoModuleBuiltins.java
@@ -51,7 +51,6 @@
 import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
 import com.oracle.graal.python.builtins.objects.dict.PDict;
 import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum;
-import com.oracle.graal.python.lib.PyObjectHashNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
 import com.oracle.graal.python.runtime.object.PFactory;
 import com.oracle.truffle.api.dsl.NodeFactory;
@@ -82,6 +81,6 @@ public void initialize(Python3Core core) {
 
     private void addConstant(int number, TruffleString name, EconomicMapStorage storage) {
         addBuiltinConstant(name, number);
-        storage.putUncachedWithJavaEq(number, PyObjectHashNode.hash(number), name);
+        storage.putUncached(number, name);
     }
 }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java
@@ -2874,8 +2874,8 @@ abstract static class SysconfNode extends PythonUnaryBuiltinNode {
         public static final EconomicMapStorage SYSCONF_NAMES = EconomicMapStorage.create();
         static {
             // TODO populate from constants
-            SYSCONF_NAMES.putUncachedWithJavaEq(T_SC_CLK_TCK, SC_CLK_TCK);
-            SYSCONF_NAMES.putUncachedWithJavaEq(T_SC_NPROCESSORS_ONLN, SC_NPROCESSORS_ONLN);
+            SYSCONF_NAMES.putUncached(T_SC_CLK_TCK, SC_CLK_TCK);
+            SYSCONF_NAMES.putUncached(T_SC_NPROCESSORS_ONLN, SC_NPROCESSORS_ONLN);
         }
 
         @Specialization
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/SysModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/SysModuleBuiltins.java
@@ -686,7 +686,7 @@ public void postInitialize0(Python3Core core) {
     private static PFrozenSet createStdLibModulesSet(PythonLanguage language) {
         EconomicMapStorage storage = EconomicMapStorage.create(STDLIB_MODULE_NAMES.length);
         for (String s : STDLIB_MODULE_NAMES) {
-            storage.putUncachedWithJavaEq(s, PNone.NONE);
+            storage.putUncached(s, PNone.NONE);
         }
         return PFactory.createFrozenSet(language, storage);
     }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/hashlib/HashlibModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/hashlib/HashlibModuleBuiltins.java
@@ -157,7 +157,7 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
     public void initialize(Python3Core core) {
         EconomicMapStorage algos = EconomicMapStorage.create(DIGEST_ALGORITHMS.length);
         for (var digest : DIGEST_ALGORITHMS) {
-            algos.putUncachedWithJavaEq(digest, PNone.NONE);
+            algos.putUncached(digest, PNone.NONE);
         }
         PythonLanguage language = core.getLanguage();
         addBuiltinConstant("openssl_md_meth_names", PFactory.createFrozenSet(language, algos));
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/EconomicMapStorage.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/EconomicMapStorage.java
@@ -155,23 +155,20 @@ public void putUncached(TruffleString key, Object value) {
         PutUnsafeNode.putUncached(this.map, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
     }
 
-    private void putUncached(Object key, Object value) {
-        PutNode.getUncached().put(null, null, this.map, key, PyObjectHashNode.executeUncached(key), value);
-    }
-
-    // Solves boot-order problem, do not use in normal code or during startup when __eq__ of
-    // builtins may not be properly set-up
-    public void putUncachedWithJavaEq(Object key, long keyHash, Object value) {
-        PutUnsafeNode.putUncachedWithJavaEq(this.map, key, keyHash, value);
+    @TruffleBoundary
+    public void putUncached(Object key, Object value) {
+        PutNode.getUncached().execute(null, null, this.map, key, PyObjectHashNode.executeUncached(key), value);
     }
 
-    public void putUncachedWithJavaEq(TruffleString key, Object value) {
-        putUncachedWithJavaEq(key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
+    @TruffleBoundary
+    public void putUncached(int key, Object value) {
+        PutUnsafeNode.putUncached(this.map, key, PyObjectHashNode.hash(key), value);
     }
 
-    public void putUncachedWithJavaEq(String key, Object value) {
+    @TruffleBoundary
+    public void putUncached(String key, Object value) {
         TruffleString ts = toTruffleStringUncached(key);
-        putUncachedWithJavaEq(ts, value);
+        putUncached(ts, value);
     }
 
     private static void putAllUncached(LinkedHashMap<String, Object> map, EconomicMapStorage result) {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/ObjectHashMap.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/ObjectHashMap.java
@@ -44,16 +44,12 @@
 
 import java.util.Arrays;
 
-import com.oracle.graal.python.builtins.PythonBuiltinClassType;
-import com.oracle.graal.python.builtins.objects.cext.PythonNativeClass;
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMapFactory.IsSideEffectingKeyNodeGen;
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMapFactory.PutNodeGen;
 import com.oracle.graal.python.builtins.objects.str.PString;
-import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
 import com.oracle.graal.python.lib.PyObjectRichCompareBool;
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.util.PythonUtils;
-import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff;
@@ -621,14 +617,6 @@ public static void putUncached(ObjectHashMap map, Object key, long keyHash, Obje
 
         abstract void execute(Frame frame, Node inliningTarget, ObjectHashMap map, Object key, long keyHash, Object value);
 
-        static void putUncachedWithJavaEq(ObjectHashMap map, Object key, long keyHash, Object value) {
-            assert isJavaEqualsAllowed(key) : key;
-            doPutWithRestart(null, null, map, key, keyHash, value,
-                            InlinedBranchProfile.getUncached(), InlinedCountingConditionProfile.getUncached(), InlinedCountingConditionProfile.getUncached(),
-                            InlinedCountingConditionProfile.getUncached(), InlinedCountingConditionProfile.getUncached(), InlinedBranchProfile.getUncached(), InlinedBranchProfile.getUncached(),
-                            null);
-        }
-
         // "public" for testing...
         @Specialization
         public static void doPutWithRestart(Frame frame, Node inliningTarget, ObjectHashMap map, Object key, long keyHash, Object value,
@@ -908,10 +896,6 @@ private boolean keysEqual(int[] originalIndices, Frame frame, Node inliningTarge
         if (originalKey == key) {
             return true;
         }
-        if (CompilerDirectives.inInterpreter() && eqNode == null) {
-            // this is hack, see putUncachedWithJavaEq
-            return javaEquals(originalKey, key);
-        }
         boolean result = eqNode.executeEq(frame, inliningTarget, originalKey, key);
         if (getKey(index) != originalKey || indices != originalIndices) {
             // Either someone overridden the slot we are just examining, or rehasing reallocated the
@@ -930,18 +914,6 @@ private boolean keysEqual(int[] originalIndices, Frame frame, Node inliningTarge
         return result;
     }
 
-    private static boolean javaEquals(Object a, Object b) {
-        CompilerAsserts.neverPartOfCompilation();
-        assert isJavaEqualsAllowed(a) : a;
-        assert isJavaEqualsAllowed(b) : b;
-        return a.equals(b);
-    }
-
-    private static boolean isJavaEqualsAllowed(Object o) {
-        return o instanceof PythonManagedClass || o instanceof PythonBuiltinClassType || //
-                        o instanceof PythonNativeClass || o instanceof Number || o instanceof TruffleString;
-    }
-
     /**
      * Called when we need space for new entry. It determines the new size from the number of slots
      * occupied by real values (i.e., does not count dummy entries), so the new size may be actually
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/BaseExceptionGroupBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/BaseExceptionGroupBuiltins.java
@@ -133,7 +133,7 @@ private static void createExceptionGroupType(Python3Core core) {
         Object typeBuiltin = builtins.getAttribute(T_TYPE);
         PTuple bases = PFactory.createTuple(language, new Object[]{PythonBuiltinClassType.PBaseExceptionGroup, PythonBuiltinClassType.Exception});
         EconomicMapStorage dictStorage = EconomicMapStorage.create(1);
-        dictStorage.putUncachedWithJavaEq(T___MODULE__, T_BUILTINS);
+        dictStorage.putUncached(T___MODULE__, T_BUILTINS);
         PDict dict = PFactory.createDict(language, dictStorage);
         Object exceptionGroupType = CallNode.executeUncached(typeBuiltin, T_EXCEPTION_GROUP, bases, dict);
         builtins.setAttribute(T_EXCEPTION_GROUP, exceptionGroupType);
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java
@@ -815,19 +815,9 @@ protected static void unsafeAddSubclass(Object base, Object subclass) {
             long hash = ObjectBuiltins.HashNode.hash(subclass);
             PDict dict = executeUncached(base);
             HashingStorage storage = dict.getDictStorage();
-            // Booting order problem: special method slot for object.__eq__ is not initialized yet
-            // In the unlikely event of hash collision __eq__ would fail
-            if (HashingStorageLen.executeUncached(storage) == 0) {
-                // This should not call __eq__
-                HashingStorageSetItemWithHash setItem = HashingStorageSetItemWithHashNodeGen.getUncached();
-                storage = setItem.execute(null, null, storage, subclass, hash, subclass);
-                dict.setDictStorage(storage);
-            } else {
-                // the storage must be EconomicMap, because keys should not be Strings so there is
-                // no other option left
-                EconomicMapStorage mapStorage = (EconomicMapStorage) storage;
-                mapStorage.putUncachedWithJavaEq(subclass, hash, subclass);
-            }
+            HashingStorageSetItemWithHash setItem = HashingStorageSetItemWithHashNodeGen.getUncached();
+            storage = setItem.execute(null, null, storage, subclass, hash, subclass);
+            dict.setDictStorage(storage);
         }
 
         protected static void unsafeRemoveSubclass(Object base, Object subclass) {

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,6 @@`
`51`	`51`	`import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;`
`52`	`52`	`import com.oracle.graal.python.builtins.objects.dict.PDict;`
`53`	`53`	`import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum;`
`54`		`-import com.oracle.graal.python.lib.PyObjectHashNode;`
`55`	`54`	`import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;`
`56`	`55`	`import com.oracle.graal.python.runtime.object.PFactory;`
`57`	`56`	`import com.oracle.truffle.api.dsl.NodeFactory;`
`@@ -82,6 +81,6 @@ public void initialize(Python3Core core) {`
`82`	`81`
`83`	`82`	`private void addConstant(int number, TruffleString name, EconomicMapStorage storage) {`
`84`	`83`	`addBuiltinConstant(name, number);`
`85`		`- storage.putUncachedWithJavaEq(number, PyObjectHashNode.hash(number), name);`
	`84`	`+ storage.putUncached(number, name);`
`86`	`85`	`}`
`87`	`86`	`}`
Original file line number	Diff line number	Diff line change
`@@ -686,7 +686,7 @@ public void postInitialize0(Python3Core core) {`
`686`	`686`	`private static PFrozenSet createStdLibModulesSet(PythonLanguage language) {`
`687`	`687`	`EconomicMapStorage storage = EconomicMapStorage.create(STDLIB_MODULE_NAMES.length);`
`688`	`688`	`for (String s : STDLIB_MODULE_NAMES) {`
`689`		`- storage.putUncachedWithJavaEq(s, PNone.NONE);`
	`689`	`+ storage.putUncached(s, PNone.NONE);`
`690`	`690`	`}`
`691`	`691`	`return PFactory.createFrozenSet(language, storage);`
`692`	`692`	`}`
Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa`
`157`	`157`	`public void initialize(Python3Core core) {`
`158`	`158`	`EconomicMapStorage algos = EconomicMapStorage.create(DIGEST_ALGORITHMS.length);`
`159`	`159`	`for (var digest : DIGEST_ALGORITHMS) {`
`160`		`- algos.putUncachedWithJavaEq(digest, PNone.NONE);`
	`160`	`+ algos.putUncached(digest, PNone.NONE);`
`161`	`161`	`}`
`162`	`162`	`PythonLanguage language = core.getLanguage();`
`163`	`163`	`addBuiltinConstant("openssl_md_meth_names", PFactory.createFrozenSet(language, algos));`