Skip to content

gh-92810: Reduce memory usage by ABCMeta.__subclasscheck__ #131914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ extern "C" {
_Py_atomic_load_uint16_relaxed(&value)
#define FT_ATOMIC_LOAD_UINT32_RELAXED(value) \
_Py_atomic_load_uint32_relaxed(&value)
#define FT_ATOMIC_LOAD_UINT64_RELAXED(value) \
_Py_atomic_load_uint64_relaxed(&value)
#define FT_ATOMIC_LOAD_ULONG_RELAXED(value) \
_Py_atomic_load_ulong_relaxed(&value)
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
Expand All @@ -61,6 +63,8 @@ extern "C" {
_Py_atomic_store_uint16_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \
_Py_atomic_store_uint32_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_UINT64_RELAXED(value, new_value) \
_Py_atomic_store_uint64_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) \
_Py_atomic_store_char_relaxed(&value, new_value)
#define FT_ATOMIC_LOAD_CHAR_RELAXED(value) \
Expand Down Expand Up @@ -111,6 +115,8 @@ extern "C" {
_Py_atomic_load_ullong_relaxed(&value)
#define FT_ATOMIC_ADD_SSIZE(value, new_value) \
(void)_Py_atomic_add_ssize(&value, new_value)
#define FT_ATOMIC_ADD_UINT64(value, new_value) \
(void)_Py_atomic_add_uint64(&value, new_value)

#else
#define FT_ATOMIC_LOAD_PTR(value) value
Expand All @@ -126,6 +132,7 @@ extern "C" {
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value
#define FT_ATOMIC_LOAD_UINT16_RELAXED(value) value
#define FT_ATOMIC_LOAD_UINT32_RELAXED(value) value
#define FT_ATOMIC_LOAD_UINT64_RELAXED(value) value
#define FT_ATOMIC_LOAD_ULONG_RELAXED(value) value
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
Expand All @@ -134,6 +141,7 @@ extern "C" {
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT64_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_LOAD_CHAR_RELAXED(value) value
#define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) value
Expand All @@ -159,6 +167,7 @@ extern "C" {
#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) value
#define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_ADD_SSIZE(value, new_value) (void)(value += new_value)
#define FT_ATOMIC_ADD_UINT64(value, new_value) (void)(value += new_value)

#endif

Expand Down
24 changes: 20 additions & 4 deletions Lib/_py_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def __new__(mcls, name, bases, namespace, /, **kwargs):
cls._abc_cache = WeakSet()
cls._abc_negative_cache = WeakSet()
cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter
cls._abc_issubclasscheck_recursive = False
return cls

def register(cls, subclass):
Expand All @@ -66,7 +67,7 @@ def register(cls, subclass):
# This would create a cycle, which is bad for the algorithm below
raise RuntimeError("Refusing to create an inheritance cycle")
cls._abc_registry.add(subclass)
ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache
ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache
return subclass

def _dump_registry(cls, file=None):
Expand Down Expand Up @@ -139,9 +140,24 @@ def __subclasscheck__(cls, subclass):
return True
# Check if it's a subclass of a subclass (recursive)
for scls in cls.__subclasses__():
if issubclass(subclass, scls):
cls._abc_cache.add(subclass)
# If inside recursive issubclass check, avoid adding classes
# to any cache because this may drastically increase memory usage.
# Unfortunately, issubclass/__subclasscheck__ don't accept third
# argument with context, so using global context within ABCMeta.
# This is done only on first method call, next will use cache.
scls_is_abc = hasattr(scls, "_abc_issubclasscheck_recursive")
if scls_is_abc:
scls._abc_issubclasscheck_recursive = True
try:
result = issubclass(subclass, scls)
finally:
if scls_is_abc:
scls._abc_issubclasscheck_recursive = False
if result:
if not cls._abc_issubclasscheck_recursive:
cls._abc_cache.add(subclass)
return True
# No dice; update negative cache
cls._abc_negative_cache.add(subclass)
if not cls._abc_issubclasscheck_recursive:
cls._abc_negative_cache.add(subclass)
return False
166 changes: 136 additions & 30 deletions Lib/test/test_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,29 +270,102 @@ def x(self):
class C(metaclass=meta):
pass

def test_isinstance_direct_inheritance(self):
class A(metaclass=abc_ABCMeta):
pass
class B(A):
pass
class C(A):
pass

a = A()
b = B()
c = C()
# trigger caching
for _ in range(2):
self.assertIsInstance(a, A)
self.assertIsInstance(a, (A,))
self.assertNotIsInstance(a, B)
self.assertNotIsInstance(a, (B,))
self.assertNotIsInstance(a, C)
self.assertNotIsInstance(a, (C,))

self.assertIsInstance(b, B)
self.assertIsInstance(b, (B,))
self.assertIsInstance(b, A)
self.assertIsInstance(b, (A,))
self.assertNotIsInstance(b, C)
self.assertNotIsInstance(b, (C,))

self.assertIsInstance(c, C)
self.assertIsInstance(c, (C,))
self.assertIsInstance(c, A)
self.assertIsInstance(c, (A,))
self.assertNotIsInstance(c, B)
self.assertNotIsInstance(c, (B,))

self.assertIsSubclass(B, A)
self.assertIsSubclass(B, (A,))
self.assertIsSubclass(C, A)
self.assertIsSubclass(C, (A,))
self.assertNotIsSubclass(B, C)
self.assertNotIsSubclass(B, (C,))
self.assertNotIsSubclass(C, B)
self.assertNotIsSubclass(C, (B,))
self.assertNotIsSubclass(A, B)
self.assertNotIsSubclass(A, (B,))
self.assertNotIsSubclass(A, C)
self.assertNotIsSubclass(A, (C,))

def test_registration_basics(self):
class A(metaclass=abc_ABCMeta):
pass
class B(object):
pass

a = A()
b = B()
self.assertNotIsSubclass(B, A)
self.assertNotIsSubclass(B, (A,))
self.assertNotIsInstance(b, A)
self.assertNotIsInstance(b, (A,))
# trigger caching
for _ in range(2):
self.assertNotIsSubclass(B, A)
self.assertNotIsSubclass(B, (A,))
self.assertNotIsInstance(b, A)
self.assertNotIsInstance(b, (A,))

self.assertNotIsSubclass(A, B)
self.assertNotIsSubclass(A, (B,))
self.assertNotIsInstance(a, B)
self.assertNotIsInstance(a, (B,))

B1 = A.register(B)
self.assertIsSubclass(B, A)
self.assertIsSubclass(B, (A,))
self.assertIsInstance(b, A)
self.assertIsInstance(b, (A,))
self.assertIs(B1, B)
# trigger caching
for _ in range(2):
self.assertIsSubclass(B, A)
self.assertIsSubclass(B, (A,))
self.assertIsInstance(b, A)
self.assertIsInstance(b, (A,))
self.assertIs(B1, B)

self.assertNotIsSubclass(A, B)
self.assertNotIsSubclass(A, (B,))
self.assertNotIsInstance(a, B)
self.assertNotIsInstance(a, (B,))

class C(B):
pass

c = C()
self.assertIsSubclass(C, A)
self.assertIsSubclass(C, (A,))
self.assertIsInstance(c, A)
self.assertIsInstance(c, (A,))
# trigger caching
for _ in range(2):
self.assertIsSubclass(C, A)
self.assertIsSubclass(C, (A,))
self.assertIsInstance(c, A)
self.assertIsInstance(c, (A,))

self.assertNotIsSubclass(A, C)
self.assertNotIsSubclass(A, (C,))
self.assertNotIsInstance(a, C)
self.assertNotIsInstance(a, (C,))

def test_register_as_class_deco(self):
class A(metaclass=abc_ABCMeta):
Expand Down Expand Up @@ -377,39 +450,73 @@ class A(metaclass=abc_ABCMeta):
pass
self.assertIsSubclass(A, A)
self.assertIsSubclass(A, (A,))

class B(metaclass=abc_ABCMeta):
pass
self.assertNotIsSubclass(A, B)
self.assertNotIsSubclass(A, (B,))
self.assertNotIsSubclass(B, A)
self.assertNotIsSubclass(B, (A,))

class C(metaclass=abc_ABCMeta):
pass
A.register(B)
class B1(B):
pass
self.assertIsSubclass(B1, A)
self.assertIsSubclass(B1, (A,))
# trigger caching
for _ in range(2):
self.assertIsSubclass(B1, A)
self.assertIsSubclass(B1, (A,))

class C1(C):
pass
B1.register(C1)
self.assertNotIsSubclass(C, B)
self.assertNotIsSubclass(C, (B,))
self.assertNotIsSubclass(C, B1)
self.assertNotIsSubclass(C, (B1,))
self.assertIsSubclass(C1, A)
self.assertIsSubclass(C1, (A,))
self.assertIsSubclass(C1, B)
self.assertIsSubclass(C1, (B,))
self.assertIsSubclass(C1, B1)
self.assertIsSubclass(C1, (B1,))
# trigger caching
for _ in range(2):
self.assertNotIsSubclass(C, B)
self.assertNotIsSubclass(C, (B,))
self.assertNotIsSubclass(C, B1)
self.assertNotIsSubclass(C, (B1,))
self.assertIsSubclass(C1, A)
self.assertIsSubclass(C1, (A,))
self.assertIsSubclass(C1, B)
self.assertIsSubclass(C1, (B,))
self.assertIsSubclass(C1, B1)
self.assertIsSubclass(C1, (B1,))

C1.register(int)
class MyInt(int):
pass
self.assertIsSubclass(MyInt, A)
self.assertIsSubclass(MyInt, (A,))
self.assertIsInstance(42, A)
self.assertIsInstance(42, (A,))
# trigger caching
for _ in range(2):
self.assertIsSubclass(MyInt, A)
self.assertIsSubclass(MyInt, (A,))
self.assertIsInstance(42, A)
self.assertIsInstance(42, (A,))

def test_custom_subclasses(self):
class A: pass
class B: pass

class Parent1(metaclass=abc_ABCMeta):
@classmethod
def __subclasses__(cls):
return [A]

class Parent2(metaclass=abc_ABCMeta):
__subclasses__ = lambda: [A]

# trigger caching
for _ in range(2):
self.assertIsInstance(A(), Parent1)
self.assertIsSubclass(A, Parent1)
self.assertNotIsInstance(B(), Parent1)
self.assertNotIsSubclass(B, Parent1)

self.assertIsInstance(A(), Parent2)
self.assertIsSubclass(A, Parent2)
self.assertNotIsInstance(B(), Parent2)
self.assertNotIsSubclass(B, Parent2)

def test_issubclass_bad_arguments(self):
class A(metaclass=abc_ABCMeta):
Expand Down Expand Up @@ -522,7 +629,6 @@ def foo(self):
self.assertEqual(A.__abstractmethods__, set())
A()


def test_update_new_abstractmethods(self):
class A(metaclass=abc_ABCMeta):
@abc.abstractmethod
Expand Down
22 changes: 22 additions & 0 deletions Lib/test/test_isinstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,28 @@ class B:
with support.infinite_recursion(25):
self.assertRaises(RecursionError, issubclass, X(), int)

def test_custom_subclasses_are_ignored(self):
class A: pass
class B: pass

class Parent1:
@classmethod
def __subclasses__(cls):
return [A, B]

class Parent2:
__subclasses__ = lambda: [A, B]

self.assertNotIsInstance(A(), Parent1)
self.assertNotIsInstance(B(), Parent1)
self.assertNotIsSubclass(A, Parent1)
self.assertNotIsSubclass(B, Parent1)

self.assertNotIsInstance(A(), Parent2)
self.assertNotIsInstance(B(), Parent2)
self.assertNotIsSubclass(A, Parent2)
self.assertNotIsSubclass(B, Parent2)


def blowstack(fxn, arg, compare_to):
# Make sure that calling isinstance with a deeply nested tuple for its
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Reduce memory usage by :meth:`~type.__subclasscheck__`
for :class:`abc.ABCMeta` and large class trees.
Loading
Loading