Skip to content

Commit ad9fb04

Browse files
timfeltimfel
committed
[GR-67183] Make subclasses weakly held
1 parent fa7e2f2 commit ad9fb04

File tree

9 files changed

+88
-103
lines changed

9 files changed

+88
-103
lines changed

‎graalpython/com.oracle.graal.python.cext/src/typeobject.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7650,7 +7650,7 @@ type_ready(PyTypeObject *type, int rerunbuiltin)
76507650
assert(_PyType_CheckConsistency(type));
76517651

76527652
// GraalPy change: for reason, see first call to Py_INCREF in this function
7653-
Py_DECREF(type);
7653+
Py_DECREF(type);
76547654
return 0;
76557655

76567656
error:
@@ -7724,8 +7724,7 @@ add_subclass(PyTypeObject *base, PyTypeObject *type)
77247724
if (key == NULL)
77257725
return -1;
77267726

7727-
// PyObject *ref = PyWeakref_NewRef((PyObject *)type, NULL);
7728-
PyObject *ref = Py_NewRef((PyObject *)type);
7727+
PyObject *ref = PyWeakref_NewRef((PyObject *)type, NULL);
77297728
if (ref == NULL) {
77307729
Py_DECREF(key);
77317730
return -1;

‎graalpython/com.oracle.graal.python.test/src/tests/test_class.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
1+
# Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
22
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
33
#
44
# The Universal Permissive License (UPL), Version 1.0
@@ -121,3 +121,18 @@ class notAMeta(metaclass=Meta):
121121
# ignore this.
122122
# assert notAMeta.metatype is NewDescriptor
123123
# assert notAMeta2.metatype is NewDescriptor
124+
125+
126+
def test_subclasses_collection():
127+
class A():
128+
pass
129+
130+
for i in range(1, 100):
131+
import gc
132+
class B(A):
133+
pass
134+
gc.collect()
135+
if len(A.__subclasses__()) < i:
136+
break
137+
138+
assert len(A.__subclasses__()) < i, len(A.__subclasses__())

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/PythonAbstractNativeObject.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,6 @@ public int compareTo(Object o) {
116116
return 0;
117117
}
118118

119-
@Override
120-
public void lookupChanged() {
121-
// TODO invalidate cached native MRO
122-
throw CompilerDirectives.shouldNotReachHere("not yet implemented");
123-
}
124-
125119
/**
126120
* For a description, see {@link #replicatedNativeReferences}.
127121
*/

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,12 @@ public void putUncached(TruffleString key, Object value) {
156156

157157
@TruffleBoundary
158158
public void putUncached(Object key, Object value) {
159-
PutNode.getUncached().execute(null, null, this.map, key, PyObjectHashNode.executeUncached(key), value);
159+
PutNode.putUncached(this.map, key, PyObjectHashNode.executeUncached(key), value);
160160
}
161161

162162
@TruffleBoundary
163163
public void putUncached(int key, Object value) {
164-
ObjectHashMap.PutNode.putUncached(this.map, key, PyObjectHashNode.hash(key), value);
164+
PutNode.putUncached(this.map, key, PyObjectHashNode.hash(key), value);
165165
}
166166

167167
@TruffleBoundary

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -47,8 +47,6 @@ public interface PythonAbstractClass extends TruffleObject {
4747

4848
public static final PythonAbstractClass[] EMPTY_ARRAY = {};
4949

50-
void lookupChanged();
51-
5250
static boolean isInstance(Object object) {
5351
return PythonManagedClass.isInstance(object) || PythonNativeClass.isInstance(object);
5452
}

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

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import com.oracle.graal.python.nodes.ErrorMessages;
4444
import com.oracle.graal.python.nodes.PGuards;
4545
import com.oracle.graal.python.nodes.PRaiseNode;
46-
import com.oracle.graal.python.runtime.PythonContext;
4746
import com.oracle.graal.python.runtime.exception.PException;
4847
import com.oracle.graal.python.runtime.object.PFactory;
4948
import com.oracle.graal.python.runtime.sequence.storage.MroSequenceStorage;
@@ -64,7 +63,6 @@ public abstract class PythonManagedClass extends PythonObject implements PythonA
6463

6564
private boolean abstractClass;
6665

67-
// Needs to maintain the order. It would make sense to use weakrefs, but CPython doesn't do that
6866
private final PDict subClasses;
6967
@CompilationFinal private Shape instanceShape;
7068
private TruffleString name;
@@ -141,20 +139,6 @@ public void invokeMro(Node node) {
141139
mroInitialized = true;
142140
}
143141

144-
/**
145-
* This method needs to be called if the mro changes. (currently not used)
146-
*/
147-
@Override
148-
public void lookupChanged() {
149-
CompilerAsserts.neverPartOfCompilation();
150-
methodResolutionOrder.lookupChanged();
151-
for (PythonAbstractClass subclass : GetSubclassesAsArrayNode.executeUncached(this)) {
152-
if (subclass != null) {
153-
subclass.lookupChanged();
154-
}
155-
}
156-
}
157-
158142
public int getIndexedSlotCount() {
159143
return indexedSlotCount;
160144
}
@@ -268,7 +252,7 @@ private void unsafeSetSuperClass(PythonAbstractClass... newBaseClasses) {
268252
Object nativeBase = PythonToNativeNodeGen.getUncached().execute(base);
269253
PCallCapiFunction.callUncached(NativeCAPISymbol.FUN_TRUFFLE_CHECK_TYPE_READY, nativeBase);
270254
}
271-
GetSubclassesNode.unsafeAddSubclass(base, this);
255+
GetSubclassesNode.addSubclass(base, this);
272256
}
273257
}
274258
}
@@ -326,25 +310,14 @@ public final void setBases(Node node, Object newBaseClass, PythonAbstractClass[]
326310

327311
if (this.baseClasses == newBaseClasses) {
328312
// take no action if bases were replaced through reentrance
329-
boolean isCtxInitialized = PythonContext.get(null).isInitialized();
330313
for (PythonAbstractClass base : oldBaseClasses) {
331314
if (base instanceof PythonManagedClass) {
332-
if (isCtxInitialized) {
333-
GetSubclassesNode.executeUncached(base).delItem(this);
334-
} else {
335-
// slots aren't populated yet during context initialization
336-
GetSubclassesNode.unsafeRemoveSubclass(base, this);
337-
}
315+
GetSubclassesNode.removeSubclass(base, this);
338316
}
339317
}
340318
for (PythonAbstractClass base : newBaseClasses) {
341319
if (base instanceof PythonManagedClass) {
342-
if (isCtxInitialized) {
343-
GetSubclassesNode.executeUncached(base).setItem(this, this);
344-
} else {
345-
// slots aren't populated yet during context initialization
346-
GetSubclassesNode.unsafeAddSubclass(base, this);
347-
}
320+
GetSubclassesNode.addSubclass(base, this);
348321
}
349322
}
350323
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -823,13 +823,11 @@ Object hook(VirtualFrame frame, Object cls, Object subclass) {
823823
@Builtin(name = J___SUBCLASSES__, minNumOfPositionalArgs = 1)
824824
@GenerateNodeFactory
825825
abstract static class SubclassesNode extends PythonUnaryBuiltinNode {
826-
826+
@TruffleBoundary
827827
@Specialization
828828
static PList getSubclasses(Object cls,
829-
@Bind Node inliningTarget,
830-
@Cached(inline = true) GetSubclassesAsArrayNode getSubclassesNode) {
831-
// TODO: missing: keep track of subclasses
832-
PythonAbstractClass[] array = getSubclassesNode.execute(inliningTarget, cls);
829+
@Bind Node inliningTarget) {
830+
PythonAbstractClass[] array = GetSubclassesAsArrayNode.executeUncached(cls);
833831
Object[] classes = new Object[array.length];
834832
PythonUtils.arraycopy(array, 0, classes, 0, array.length);
835833
return PFactory.createList(PythonLanguage.get(inliningTarget), classes);

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

Lines changed: 53 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,7 @@
133133
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorKeyHash;
134134
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorNext;
135135
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageIteratorValue;
136-
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageLen;
137136
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageSetItemWithHash;
138-
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodesFactory.HashingStorageSetItemWithHashNodeGen;
139137
import com.oracle.graal.python.builtins.objects.common.SequenceNodes.GetObjectArrayNode;
140138
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
141139
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetInternalObjectArrayNode;
@@ -155,6 +153,7 @@
155153
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins;
156154
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltinsFactory;
157155
import com.oracle.graal.python.builtins.objects.object.PythonObject;
156+
import com.oracle.graal.python.builtins.objects.referencetype.PReferenceType;
158157
import com.oracle.graal.python.builtins.objects.str.StringBuiltins.IsIdentifierNode;
159158
import com.oracle.graal.python.builtins.objects.str.StringUtils;
160159
import com.oracle.graal.python.builtins.objects.superobject.SuperObject;
@@ -806,31 +805,51 @@ static TruffleString getQualName(Node inliningTarget, PythonAbstractNativeObject
806805
}
807806

808807
@GenerateUncached
809-
@GenerateInline
810808
@GenerateCached(false)
811809
public abstract static class GetSubclassesNode extends PNodeWithContext {
812-
813-
public abstract PDict execute(Node inliningTarget, Object clazz);
810+
abstract PDict execute(Node inliningTarget, Object clazz);
814811

815812
public static PDict executeUncached(Object clazz) {
816813
return GetSubclassesNodeGen.getUncached().execute(null, clazz);
817814
}
818815

819-
protected static void unsafeAddSubclass(Object base, Object subclass) {
820-
long hash = ObjectBuiltins.HashNode.hash(subclass);
816+
protected static void addSubclass(PythonAbstractClass base, PythonManagedClass subclass) {
817+
CompilerAsserts.neverPartOfCompilation();
821818
PDict dict = executeUncached(base);
822819
HashingStorage storage = dict.getDictStorage();
823-
HashingStorageSetItemWithHash setItem = HashingStorageSetItemWithHashNodeGen.getUncached();
824-
storage = setItem.execute(null, null, storage, subclass, hash, subclass);
820+
Object weakref = PFactory.createReferenceType(PythonLanguage.get(null), subclass);
821+
if (!(storage instanceof EconomicMapStorage)) {
822+
assert storage == EmptyStorage.INSTANCE : "Unexpected storage type!";
823+
storage = EconomicMapStorage.create();
824+
}
825+
((EconomicMapStorage) storage).putUncached(weakref, weakref);
825826
dict.setDictStorage(storage);
826827
}
827828

828-
protected static void unsafeRemoveSubclass(Object base, Object subclass) {
829-
long hash = ObjectBuiltins.HashNode.hash(subclass);
829+
static final class RemoveSubclassValue extends HashingStorageForEachCallback<PythonManagedClass> {
830+
@Override
831+
public final PythonManagedClass execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, PythonManagedClass toRemove) {
832+
if (toRemove == null) {
833+
return null;
834+
}
835+
Object value = HashingStorageIteratorValue.executeUncached(storage, it);
836+
if (value instanceof PReferenceType pref) {
837+
Object subclassValue = pref.getObject();
838+
if (subclassValue == toRemove) {
839+
pref.clearRef();
840+
return null;
841+
}
842+
}
843+
return toRemove;
844+
}
845+
}
846+
847+
protected static void removeSubclass(PythonAbstractClass base, PythonManagedClass subclass) {
848+
CompilerAsserts.neverPartOfCompilation();
830849
PDict dict = executeUncached(base);
831850
HashingStorage storage = dict.getDictStorage();
832-
if (storage instanceof EconomicMapStorage ems) {
833-
HashingStorageDelItem.executeUncachedWithHash(ems, subclass, hash);
851+
if (storage instanceof EconomicMapStorage) {
852+
HashingStorageForEach.executeUncached(storage, new RemoveSubclassValue(), subclass);
834853
} else {
835854
assert storage == EmptyStorage.INSTANCE : "Unexpected storage type!";
836855
}
@@ -842,30 +861,26 @@ static PDict doPythonClass(PythonManagedClass obj) {
842861
}
843862

844863
@Specialization
845-
static PDict doPythonClass(Node inliningTarget, PythonBuiltinClassType obj) {
846-
return PythonContext.get(inliningTarget).lookupType(obj).getSubClasses();
864+
static PDict doPythonClass(PythonBuiltinClassType obj) {
865+
return PythonContext.get(null).lookupType(obj).getSubClasses();
847866
}
848867

849868
@Specialization
850-
static PDict doNativeClass(Node inliningTarget, PythonAbstractNativeObject obj,
851-
@Cached(inline = false) CStructAccess.ReadObjectNode getTpSubclassesNode,
852-
@Cached InlinedExactClassProfile profile) {
869+
static PDict doNativeClass(PythonAbstractNativeObject obj,
870+
@Cached(inline = false) CStructAccess.ReadObjectNode getTpSubclassesNode) {
853871
Object tpSubclasses = getTpSubclassesNode.readFromObj(obj, PyTypeObject__tp_subclasses);
854-
855-
Object profiled = profile.profile(inliningTarget, tpSubclasses);
872+
Object profiled = tpSubclasses;
856873
if (profiled instanceof PDict dict) {
857874
return dict;
858875
}
859-
CompilerDirectives.transferToInterpreterAndInvalidate();
860-
throw new IllegalStateException("invalid subclasses dict " + profiled.getClass().getName());
876+
throw CompilerDirectives.shouldNotReachHere("invalid subclasses dict " + profiled.getClass().getName());
861877
}
862878
}
863879

864880
@GenerateUncached
865-
@GenerateInline(true)
866-
@GenerateCached(true)
881+
@GenerateInline(false)
882+
@GenerateCached(false)
867883
public abstract static class GetSubclassesAsArrayNode extends Node {
868-
869884
private static final PythonAbstractClass[] EMPTY = new PythonAbstractClass[0];
870885

871886
abstract PythonAbstractClass[] execute(Node inliningTarget, Object clazz);
@@ -874,43 +889,29 @@ public static PythonAbstractClass[] executeUncached(Object clazz) {
874889
return GetSubclassesAsArrayNodeGen.getUncached().execute(null, clazz);
875890
}
876891

877-
static final class PythonAbstractClassList {
878-
final PythonAbstractClass[] subclasses;
879-
int i;
880-
881-
PythonAbstractClassList(PythonAbstractClass[] subclasses) {
882-
this.subclasses = subclasses;
883-
this.i = 0;
884-
}
885-
886-
void add(PythonAbstractClass clazz) {
887-
subclasses[i++] = clazz;
888-
}
889-
}
890-
891892
@GenerateUncached
892-
@GenerateInline(true)
893-
abstract static class EachSubclassAdd extends HashingStorageForEachCallback<PythonAbstractClassList> {
894-
893+
@GenerateCached(false)
894+
abstract static class EachSubclassAdd extends HashingStorageForEachCallback<ArrayList<PythonAbstractClass>> {
895895
@Override
896-
public abstract PythonAbstractClassList execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, PythonAbstractClassList subclasses);
896+
public abstract ArrayList<PythonAbstractClass> execute(Frame frame, Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ArrayList<PythonAbstractClass> subclasses);
897897

898898
@Specialization
899-
static PythonAbstractClassList doIt(Node inliningTarget, HashingStorage storage, HashingStorageIterator it, PythonAbstractClassList subclasses,
899+
static ArrayList<PythonAbstractClass> doIt(Node inliningTarget, HashingStorage storage, HashingStorageIterator it, ArrayList<PythonAbstractClass> subclasses,
900900
@Cached HashingStorageIteratorValue itValue) {
901901
Object value = itValue.execute(inliningTarget, storage, it);
902-
subclasses.add(PythonAbstractClass.cast(value));
902+
PythonAbstractClass clazz = PythonAbstractClass.cast(((PReferenceType) value).getObject());
903+
if (clazz != null) {
904+
subclasses.add(clazz);
905+
}
903906
return subclasses;
904907
}
905908
}
906909

907910
@Specialization
908-
static PythonAbstractClass[] doTpSubclasses(Node inliningTarget, Object object,
909-
@Cached GetSubclassesNode getSubclassesNode,
911+
static PythonAbstractClass[] doTpSubclasses(PythonAbstractClass object,
910912
@Cached EachSubclassAdd eachNode,
911-
@Cached HashingStorageLen dictLen,
912913
@Cached HashingStorageForEach forEachNode) {
913-
PDict subclasses = getSubclassesNode.execute(inliningTarget, object);
914+
PDict subclasses = GetSubclassesNode.executeUncached(object);
914915
if (subclasses == null) {
915916
return EMPTY;
916917
}
@@ -920,10 +921,9 @@ static PythonAbstractClass[] doTpSubclasses(Node inliningTarget, Object object,
920921
return EMPTY;
921922
}
922923

923-
int size = dictLen.execute(inliningTarget, storage);
924-
PythonAbstractClassList list = new PythonAbstractClassList(new PythonAbstractClass[size]);
925-
forEachNode.execute(null, inliningTarget, storage, eachNode, list);
926-
return list.subclasses;
924+
ArrayList<PythonAbstractClass> list = new ArrayList<>();
925+
forEachNode.execute(null, null, storage, eachNode, list);
926+
return list.toArray(EMPTY);
927927
}
928928
}
929929

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/object/PFactory.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,14 @@ public static PMappingproxy createMappingproxy(PythonLanguage language, Object c
873873
return trace(language, new PMappingproxy(cls, shape, object));
874874
}
875875

876+
public static PReferenceType createReferenceType(PythonLanguage language, Object object) {
877+
return createReferenceType(language, PythonBuiltinClassType.PReferenceType, PythonBuiltinClassType.PReferenceType.getInstanceShape(language), object);
878+
}
879+
880+
public static PReferenceType createReferenceType(PythonLanguage language, Object cls, Shape shape, Object object) {
881+
return createReferenceType(language, cls, shape, object, null, null);
882+
}
883+
876884
public static PReferenceType createReferenceType(PythonLanguage language, Object cls, Shape shape, Object object, Object callback, ReferenceQueue<Object> queue) {
877885
return trace(language, new PReferenceType(cls, shape, object, callback, queue));
878886
}

0 commit comments

Comments
 (0)