Make an effort to clear collected references from the __subclasses__ dict

Author: timfel

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

@@ -50,6 +50,7 @@
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.DictKey;
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.MapCursor;
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.PutNode;
+import com.oracle.graal.python.builtins.objects.common.ObjectHashMap.RemoveNode;
 import com.oracle.graal.python.lib.PyObjectHashNode;
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -149,19 +150,24 @@ protected void setValueForAllKeys(VirtualFrame frame, Node inliningTarget, Objec
         }
     }
 
+    @TruffleBoundary
+    public Object removeUncached(Object key, long hash) {
+        return RemoveNode.removeUncached(map, key, hash);
+    }
+
     @TruffleBoundary
     public void putUncached(TruffleString key, Object value) {
-        ObjectHashMap.PutNode.putUncached(this.map, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
+        PutNode.putUncached(map, key, PyObjectHashNode.hash(key, HashCodeNode.getUncached()), value);
     }
 
     @TruffleBoundary
     public void putUncached(Object key, Object value) {
-        PutNode.putUncached(this.map, key, PyObjectHashNode.executeUncached(key), value);
+        PutNode.putUncached(map, key, PyObjectHashNode.executeUncached(key), value);
     }
 
     @TruffleBoundary
     public void putUncached(int key, Object value) {
-        PutNode.putUncached(this.map, key, PyObjectHashNode.hash(key), value);
+        PutNode.putUncached(map, key, PyObjectHashNode.hash(key), value);
     }
 
     @TruffleBoundary

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

@@ -54,6 +54,7 @@
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageGetItemWithHashNodeGen;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageGetIteratorNodeGen;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageGetReverseIteratorNodeGen;
+import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageIteratorKeyHashNodeGen;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageIteratorKeyNodeGen;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageIteratorNextNodeGen;
 import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageIteratorValueNodeGen;
@@ -1120,6 +1121,9 @@ static Object foreign(ForeignHashingStorage self, HashingStorageIterator it,
     @GenerateInline
     @GenerateCached(false)
     public abstract static class HashingStorageIteratorKeyHash extends PNodeWithContext {
+        public static long executeUncached(HashingStorage storage, HashingStorageIterator it) {
+            return HashingStorageIteratorKeyHashNodeGen.getUncached().execute(null, null, storage, it);
+        }
 
         public abstract long execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it);
 

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

@@ -45,6 +45,7 @@
 import java.util.Arrays;
 
 import com.oracle.graal.python.builtins.objects.common.ObjectHashMapFactory.PutNodeGen;
+import com.oracle.graal.python.builtins.objects.common.ObjectHashMapFactory.RemoveNodeGen;
 import com.oracle.graal.python.lib.PyObjectRichCompareBool;
 import com.oracle.graal.python.util.PythonUtils;
 import com.oracle.truffle.api.CompilerDirectives;
@@ -677,6 +678,10 @@ private boolean needsCompaction() {
     @GenerateInline
     @GenerateCached(false)
     public abstract static class RemoveNode extends Node {
+        public static Object removeUncached(ObjectHashMap map, Object key, long keyHash) {
+            return RemoveNodeGen.getUncached().execute(null, null, map, key, keyHash);
+        }
+
         public abstract Object execute(Frame frame, Node inliningTarget, ObjectHashMap map, Object key, long keyHash);
 
         // "public" for testing...

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

@@ -803,6 +803,33 @@ static TruffleString getQualName(Node inliningTarget, PythonAbstractNativeObject
     }
 
     public static final class GetSubclassesNode {
+        static record KeyAndHash(Object key, long hash) {
+        }
+
+        static final class CollectEmptyKeys extends HashingStorageForEachCallback<List<KeyAndHash>> {
+            @Override
+            public final List<KeyAndHash> execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, List<KeyAndHash> acc) {
+                Object value = HashingStorageIteratorValue.executeUncached(storage, it);
+                if (value instanceof PReferenceType pref) {
+                    Object subclassValue = pref.getObject();
+                    if (subclassValue == null) {
+                        Object key = HashingStorageIteratorKey.executeUncached(storage, it);
+                        long hash = HashingStorageIteratorKeyHash.executeUncached(storage, it);
+                        acc.add(new KeyAndHash(key, hash));
+                    }
+                }
+                return acc;
+            }
+        }
+
+        static void clearEmptyReferences(EconomicMapStorage storage) {
+            List<KeyAndHash> acc = new ArrayList<>();
+            HashingStorageForEach.executeUncached(storage, new CollectEmptyKeys(), acc);
+            for (KeyAndHash k : acc) {
+                storage.removeUncached(k.key(), k.hash());
+            }
+        }
+
         protected static void addSubclass(PythonAbstractClass base, PythonManagedClass subclass) {
             CompilerAsserts.neverPartOfCompilation();
             PDict dict = executeUncached(base);
@@ -811,9 +838,11 @@ protected static void addSubclass(PythonAbstractClass base, PythonManagedClass s
             if (!(storage instanceof EconomicMapStorage)) {
                 assert storage == EmptyStorage.INSTANCE : "Unexpected storage type!";
                 storage = EconomicMapStorage.create();
+                dict.setDictStorage(storage);
+            } else {
+                clearEmptyReferences((EconomicMapStorage) storage);
             }
             ((EconomicMapStorage) storage).putUncached(weakref, weakref);
-            dict.setDictStorage(storage);
         }
 
         static final class RemoveSubclassValue extends HashingStorageForEachCallback<PythonManagedClass> {
@@ -838,8 +867,9 @@ protected static void removeSubclass(PythonAbstractClass base, PythonManagedClas
             CompilerAsserts.neverPartOfCompilation();
             PDict dict = executeUncached(base);
             HashingStorage storage = dict.getDictStorage();
-            if (storage instanceof EconomicMapStorage) {
-                HashingStorageForEach.executeUncached(storage, new RemoveSubclassValue(), subclass);
+            if (storage instanceof EconomicMapStorage ems) {
+                HashingStorageForEach.executeUncached(ems, new RemoveSubclassValue(), subclass);
+                clearEmptyReferences(ems);
             } else {
                 assert storage == EmptyStorage.INSTANCE : "Unexpected storage type!";
             }