Skip to content

Commit 6fc994d

Browse files
committed
gh-92810: Fixes after review
1 parent bbaf38a commit 6fc994d

File tree

4 files changed

+48
-32
lines changed

4 files changed

+48
-32
lines changed

Lib/_py_abc.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ def register(cls, subclass):
6767
# This would create a cycle, which is bad for the algorithm below
6868
raise RuntimeError("Refusing to create an inheritance cycle")
6969
cls._abc_registry.add(subclass)
70-
# Invalidate negative cache
71-
ABCMeta._abc_invalidation_counter += 1
70+
ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache
7271
return subclass
7372

7473
def _dump_registry(cls, file=None):
@@ -139,29 +138,26 @@ def __subclasscheck__(cls, subclass):
139138
if issubclass(subclass, rcls):
140139
cls._abc_cache.add(subclass)
141140
return True
142-
143141
# Check if it's a subclass of a subclass (recursive)
144142
for scls in cls.__subclasses__():
145-
# If inside recursive issubclass check, avoid adding classes to any cache because this
146-
# may drastically increase memory usage.
147-
# Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context,
148-
# so using global context within ABCMeta.
149-
# This is done only on first method call, others will use cached result.
143+
# If inside recursive issubclass check, avoid adding classes
144+
# to any cache because this may drastically increase memory usage.
145+
# Unfortunately, issubclass/__subclasscheck__ don't accept third
146+
# argument with context, so using global context within ABCMeta.
147+
# This is done only on first method call, next will use cache.
150148
scls_is_abc = hasattr(scls, "_abc_issubclasscheck_recursive")
151149
if scls_is_abc:
152150
scls._abc_issubclasscheck_recursive = True
153-
154151
try:
155152
result = issubclass(subclass, scls)
156153
finally:
157154
if scls_is_abc:
158155
scls._abc_issubclasscheck_recursive = False
159-
160156
if result:
161157
if not cls._abc_issubclasscheck_recursive:
162158
cls._abc_cache.add(subclass)
163159
return True
164-
160+
# No dice; update negative cache
165161
if not cls._abc_issubclasscheck_recursive:
166162
cls._abc_negative_cache.add(subclass)
167163
return False

Lib/test/test_abc.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ class B(A):
277277
pass
278278
class C(A):
279279
pass
280+
280281
a = A()
281282
b = B()
282283
c = C()
@@ -321,9 +322,9 @@ class A(metaclass=abc_ABCMeta):
321322
pass
322323
class B(object):
323324
pass
325+
324326
a = A()
325327
b = B()
326-
327328
# trigger caching
328329
for _ in range(2):
329330
self.assertNotIsSubclass(B, A)
@@ -352,6 +353,7 @@ class B(object):
352353

353354
class C(B):
354355
pass
356+
355357
c = C()
356358
# trigger caching
357359
for _ in range(2):
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
Reduce memory usage by :meth:`~type.__subclasscheck__`
2-
for :class:`abc.ABCMeta` and large class trees
2+
for :class:`abc.ABCMeta` and large class trees.

Modules/_abc.c

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ typedef struct {
6565
PyObject *_abc_cache;
6666
PyObject *_abc_negative_cache;
6767
uint64_t _abc_negative_cache_version;
68-
bool _abc_issubclasscheck_recursive;
68+
uint8_t _abc_issubclasscheck_recursive;
6969
} _abc_data;
7070

7171
#define _abc_data_CAST(op) ((_abc_data *)(op))
@@ -90,22 +90,34 @@ set_cache_version(_abc_data *impl, uint64_t version)
9090
#endif
9191
}
9292

93-
static inline bool
93+
static inline uint8_t
9494
is_issubclasscheck_recursive(_abc_data *impl)
9595
{
96+
#ifdef Py_GIL_DISABLED
97+
return _Py_atomic_load_uint64(&impl->_abc_issubclasscheck_recursive);
98+
#else
9699
return impl->_abc_issubclasscheck_recursive;
100+
#endif
97101
}
98102

99103
static inline void
100104
set_issubclasscheck_recursive(_abc_data *impl)
101105
{
102-
impl->_abc_issubclasscheck_recursive = true;
106+
#ifdef Py_GIL_DISABLED
107+
_Py_atomic_store_uint8(&impl->_abc_issubclasscheck_recursive, 1);
108+
#else
109+
impl->_abc_issubclasscheck_recursive = 1;
110+
#endif
103111
}
104112

105113
static inline void
106114
unset_issubclasscheck_recursive(_abc_data *impl)
107115
{
108-
impl->_abc_issubclasscheck_recursive = false;
116+
#ifdef Py_GIL_DISABLED
117+
_Py_atomic_store_uint8(&impl->_abc_issubclasscheck_recursive, 0);
118+
#else
119+
impl->_abc_issubclasscheck_recursive = 0;
120+
#endif
109121
}
110122

111123
static int
@@ -158,7 +170,7 @@ abc_data_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
158170
self->_abc_cache = NULL;
159171
self->_abc_negative_cache = NULL;
160172
self->_abc_negative_cache_version = get_invalidation_counter(state);
161-
self->_abc_issubclasscheck_recursive = false;
173+
self->_abc_issubclasscheck_recursive = 0;
162174
return (PyObject *) self;
163175
}
164176

@@ -197,21 +209,24 @@ _get_impl(PyObject *module, PyObject *self)
197209
return (_abc_data *)impl;
198210
}
199211

200-
static _abc_data *
201-
_get_impl_optional(PyObject *module, PyObject *self)
212+
static int
213+
_get_impl_optional(PyObject *module, PyObject *self, _abc_data **data)
202214
{
203215
_abcmodule_state *state = get_abc_state(module);
204216
PyObject *impl = NULL;
205217
int res = PyObject_GetOptionalAttr(self, &_Py_ID(_abc_impl), &impl);
206218
if (res <= 0) {
207-
return NULL;
219+
*data = NULL;
220+
return res;
208221
}
209222
if (!Py_IS_TYPE(impl, state->_abc_data_type)) {
210223
PyErr_SetString(PyExc_TypeError, "_abc_impl is set to a wrong type");
211224
Py_DECREF(impl);
212-
return NULL;
225+
*data = NULL;
226+
return -1;
213227
}
214-
return (_abc_data *)impl;
228+
*data = (_abc_data *)impl;
229+
return 1;
215230
}
216231

217232
static int
@@ -853,22 +868,25 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
853868
goto end;
854869
}
855870

856-
_abc_data *scls_impl = _get_impl_optional(module, scls);
857-
if (scls_impl != NULL) {
871+
_abc_data *scls_impl;
872+
int scls_is_abc = _get_impl_optional(module, scls, &scls_impl);
873+
if (scls_is_abc < 0) {
874+
goto end;
875+
}
876+
if (scls_is_abc > 0) {
858877
/*
859-
If inside recursive issubclass check, avoid adding classes to any cache because this
860-
may drastically increase memory usage.
861-
Unfortunately, issubclass/__subclasscheck__ don't accept third argument with context,
862-
so using global context within ABCMeta.
863-
This is done only on first method call, others will use cached result.
878+
If inside recursive issubclass check, avoid adding classes
879+
to any cache because this may drastically increase memory usage.
880+
Unfortunately, issubclass/__subclasscheck__ don't accept third
881+
argument with context, so using global context within ABCMeta.
882+
This is done only on first method call, next will use cache.
864883
*/
865884
set_issubclasscheck_recursive(scls_impl);
866885
}
867886

868887
int r = PyObject_IsSubclass(subclass, scls);
869888
Py_DECREF(scls);
870-
871-
if (scls_impl != NULL) {
889+
if (scls_is_abc > 0) {
872890
unset_issubclasscheck_recursive(scls_impl);
873891
Py_DECREF(scls_impl);
874892
}

0 commit comments

Comments
 (0)