From de64e7561680fdc5358001e9488091e75d4174a3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 2 May 2023 21:30:03 -0600 Subject: gh-94673: More Per-Interpreter Fields for Builtin Static Types (gh-103912) his involves moving tp_dict, tp_bases, and tp_mro to PyInterpreterState, in the same way we did for tp_subclasses. Those three fields are effectively const for builtin static types (unlike tp_subclasses). In theory we only need to make their values immortal, along with their contents. However, that isn't such a simple proposition. (See gh-103823.) In the meantime the simplest solution is to move the fields into the interpreter. One alternative is to statically allocate the values, but that's its own can of worms. --- Include/internal/pycore_typeobject.h | 7 ++ Modules/_abc.c | 1 + Modules/gcmodule.c | 31 +---- Objects/structseq.c | 9 +- Objects/typeobject.c | 223 +++++++++++++++++++++++++++-------- 5 files changed, 185 insertions(+), 86 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index f865e51..6a5ab7e 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -44,6 +44,13 @@ struct type_cache { typedef struct { PyTypeObject *type; + int readying; + int ready; + // XXX tp_dict, tp_bases, and tp_mro can probably be statically + // allocated, instead of dynamically and stored on the interpreter. + PyObject *tp_dict; + PyObject *tp_bases; + PyObject *tp_mro; PyObject *tp_subclasses; /* We never clean up weakrefs for static builtin types since they will effectively never get triggered. However, there diff --git a/Modules/_abc.c b/Modules/_abc.c index 997b618..9694331 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -7,6 +7,7 @@ #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object.h" // _PyType_GetSubclasses() #include "pycore_runtime.h" // _Py_ID() +#include "pycore_typeobject.h" // _PyType_GetMRO() #include "clinic/_abc.c.h" /*[clinic input] diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 8a4d1a4..f4d5186 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -2174,23 +2174,6 @@ _PyGC_DumpShutdownStats(PyInterpreterState *interp) } -static void -gc_fini_untrack(PyGC_Head *list) -{ - PyGC_Head *gc; - for (gc = GC_NEXT(list); gc != list; gc = GC_NEXT(list)) { - PyObject *op = FROM_GC(gc); - _PyObject_GC_UNTRACK(op); - // gh-92036: If a deallocator function expect the object to be tracked - // by the GC (ex: func_dealloc()), it can crash if called on an object - // which is no longer tracked by the GC. Leak one strong reference on - // purpose so the object is never deleted and its deallocator is not - // called. - Py_INCREF(op); - } -} - - void _PyGC_Fini(PyInterpreterState *interp) { @@ -2198,17 +2181,9 @@ _PyGC_Fini(PyInterpreterState *interp) Py_CLEAR(gcstate->garbage); Py_CLEAR(gcstate->callbacks); - if (!_Py_IsMainInterpreter(interp)) { - // bpo-46070: Explicitly untrack all objects currently tracked by the - // GC. Otherwise, if an object is used later by another interpreter, - // calling PyObject_GC_UnTrack() on the object crashs if the previous - // or the next object of the PyGC_Head structure became a dangling - // pointer. - for (int i = 0; i < NUM_GENERATIONS; i++) { - PyGC_Head *gen = GEN_HEAD(gcstate, i); - gc_fini_untrack(gen); - } - } + /* We expect that none of this interpreters objects are shared + with other interpreters. + See https://github.com/python/cpython/issues/90228. */ } /* for debugging */ diff --git a/Objects/structseq.c b/Objects/structseq.c index f63660a..8b18959 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -511,7 +511,6 @@ _PyStructSequence_InitBuiltinWithFlags(PyInterpreterState *interp, Py_ssize_t n_members = count_members(desc, &n_unnamed_members); PyMemberDef *members = NULL; - int initialized = 1; if ((type->tp_flags & Py_TPFLAGS_READY) == 0) { assert(type->tp_name == NULL); assert(type->tp_members == NULL); @@ -524,7 +523,6 @@ _PyStructSequence_InitBuiltinWithFlags(PyInterpreterState *interp, initialize_static_fields(type, desc, members, tp_flags); _Py_SetImmortal(type); - initialized = 0; } #ifndef NDEBUG else { @@ -543,13 +541,10 @@ _PyStructSequence_InitBuiltinWithFlags(PyInterpreterState *interp, desc->name); goto error; } - // This should be dropped if tp_dict is made per-interpreter. - if (initialized) { - return 0; - } if (initialize_structseq_dict( - desc, _PyType_GetDict(type), n_members, n_unnamed_members) < 0) { + desc, _PyType_GetDict(type), n_members, n_unnamed_members) < 0) + { goto error; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a9d3a69..cf0efe1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -167,29 +167,93 @@ static_builtin_state_clear(PyInterpreterState *interp, PyTypeObject *self) /* end static builtin helpers */ +static inline void +start_readying(PyTypeObject *type) +{ + if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = static_builtin_state_get(interp, type); + assert(state != NULL); + assert(!state->readying); + state->readying = 1; + return; + } + assert((type->tp_flags & Py_TPFLAGS_READYING) == 0); + type->tp_flags |= Py_TPFLAGS_READYING; +} + +static inline void +stop_readying(PyTypeObject *type) +{ + if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = static_builtin_state_get(interp, type); + assert(state != NULL); + assert(state->readying); + state->readying = 0; + return; + } + assert(type->tp_flags & Py_TPFLAGS_READYING); + type->tp_flags &= ~Py_TPFLAGS_READYING; +} + +static inline int +is_readying(PyTypeObject *type) +{ + if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = static_builtin_state_get(interp, type); + assert(state != NULL); + return state->readying; + } + return (type->tp_flags & Py_TPFLAGS_READYING) != 0; +} + + /* accessors for objects stored on PyTypeObject */ static inline PyObject * lookup_tp_dict(PyTypeObject *self) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + return state->tp_dict; + } return self->tp_dict; } PyObject * _PyType_GetDict(PyTypeObject *self) { + /* It returns a borrowed reference. */ return lookup_tp_dict(self); } static inline void set_tp_dict(PyTypeObject *self, PyObject *dict) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + state->tp_dict = dict; + return; + } self->tp_dict = dict; } static inline void clear_tp_dict(PyTypeObject *self) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + Py_CLEAR(state->tp_dict); + return; + } Py_CLEAR(self->tp_dict); } @@ -197,24 +261,45 @@ clear_tp_dict(PyTypeObject *self) static inline PyObject * lookup_tp_bases(PyTypeObject *self) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + return state->tp_bases; + } return self->tp_bases; } PyObject * _PyType_GetBases(PyTypeObject *self) { + /* It returns a borrowed reference. */ return lookup_tp_bases(self); } static inline void set_tp_bases(PyTypeObject *self, PyObject *bases) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + state->tp_bases = bases; + return; + } self->tp_bases = bases; } static inline void clear_tp_bases(PyTypeObject *self) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + Py_CLEAR(state->tp_bases); + return; + } Py_CLEAR(self->tp_bases); } @@ -222,24 +307,45 @@ clear_tp_bases(PyTypeObject *self) static inline PyObject * lookup_tp_mro(PyTypeObject *self) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + return state->tp_mro; + } return self->tp_mro; } PyObject * _PyType_GetMRO(PyTypeObject *self) { + /* It returns a borrowed reference. */ return lookup_tp_mro(self); } static inline void set_tp_mro(PyTypeObject *self, PyObject *mro) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + state->tp_mro = mro; + return; + } self->tp_mro = mro; } static inline void clear_tp_mro(PyTypeObject *self) { + if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + static_builtin_state *state = _PyStaticType_GetState(interp, self); + assert(state != NULL); + Py_CLEAR(state->tp_mro); + return; + } Py_CLEAR(self->tp_mro); } @@ -408,7 +514,7 @@ _PyType_CheckConsistency(PyTypeObject *type) CHECK(Py_REFCNT(type) >= 1); CHECK(PyType_Check(type)); - CHECK(!(type->tp_flags & Py_TPFLAGS_READYING)); + CHECK(!is_readying(type)); CHECK(lookup_tp_dict(type) != NULL); if (type->tp_flags & Py_TPFLAGS_HAVE_GC) { @@ -809,7 +915,6 @@ static PyMemberDef type_members[] = { {"__base__", T_OBJECT, offsetof(PyTypeObject, tp_base), READONLY}, {"__dictoffset__", T_PYSSIZET, offsetof(PyTypeObject, tp_dictoffset), READONLY}, - {"__mro__", T_OBJECT, offsetof(PyTypeObject, tp_mro), READONLY}, {0} }; @@ -1023,7 +1128,21 @@ type_set_abstractmethods(PyTypeObject *type, PyObject *value, void *context) static PyObject * type_get_bases(PyTypeObject *type, void *context) { - return Py_NewRef(lookup_tp_bases(type)); + PyObject *bases = lookup_tp_bases(type); + if (bases == NULL) { + Py_RETURN_NONE; + } + return Py_NewRef(bases); +} + +static PyObject * +type_get_mro(PyTypeObject *type, void *context) +{ + PyObject *mro = lookup_tp_mro(type); + if (mro == NULL) { + Py_RETURN_NONE; + } + return Py_NewRef(mro); } static PyTypeObject *best_base(PyObject *); @@ -1402,6 +1521,7 @@ static PyGetSetDef type_getsets[] = { {"__name__", (getter)type_name, (setter)type_set_name, NULL}, {"__qualname__", (getter)type_qualname, (setter)type_set_qualname, NULL}, {"__bases__", (getter)type_get_bases, (setter)type_set_bases, NULL}, + {"__mro__", (getter)type_get_mro, NULL, NULL}, {"__module__", (getter)type_module, (setter)type_set_module, NULL}, {"__abstractmethods__", (getter)type_abstractmethods, (setter)type_set_abstractmethods, NULL}, @@ -4342,7 +4462,7 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) /* Look in tp_dict of types in MRO */ PyObject *mro = lookup_tp_mro(type); if (mro == NULL) { - if ((type->tp_flags & Py_TPFLAGS_READYING) == 0) { + if (!is_readying(type)) { if (PyType_Ready(type) < 0) { *error = -1; return NULL; @@ -4692,11 +4812,11 @@ static void clear_static_type_objects(PyInterpreterState *interp, PyTypeObject *type) { if (_Py_IsMainInterpreter(interp)) { - clear_tp_dict(type); - clear_tp_bases(type); - clear_tp_mro(type); Py_CLEAR(type->tp_cache); } + clear_tp_dict(type); + clear_tp_bases(type); + clear_tp_mro(type); clear_static_tp_subclasses(type); } @@ -6684,6 +6804,10 @@ type_ready_pre_checks(PyTypeObject *type) static int type_ready_set_bases(PyTypeObject *type) { + if (lookup_tp_bases(type) != NULL) { + return 0; + } + /* Initialize tp_base (defaults to BaseObject unless that's us) */ PyTypeObject *base = type->tp_base; if (base == NULL && type != &PyBaseObject_Type) { @@ -6997,7 +7121,7 @@ type_ready_add_subclasses(PyTypeObject *type) // Set tp_new and the "__new__" key in the type dictionary. // Use the Py_TPFLAGS_DISALLOW_INSTANTIATION flag. static int -type_ready_set_new(PyTypeObject *type) +type_ready_set_new(PyTypeObject *type, int rerunbuiltin) { PyTypeObject *base = type->tp_base; /* The condition below could use some explanation. @@ -7019,10 +7143,12 @@ type_ready_set_new(PyTypeObject *type) if (!(type->tp_flags & Py_TPFLAGS_DISALLOW_INSTANTIATION)) { if (type->tp_new != NULL) { - // If "__new__" key does not exists in the type dictionary, - // set it to tp_new_wrapper(). - if (add_tp_new_wrapper(type) < 0) { - return -1; + if (!rerunbuiltin || base == NULL || type->tp_new != base->tp_new) { + // If "__new__" key does not exists in the type dictionary, + // set it to tp_new_wrapper(). + if (add_tp_new_wrapper(type) < 0) { + return -1; + } } } else { @@ -7096,11 +7222,10 @@ type_ready_post_checks(PyTypeObject *type) static int -type_ready(PyTypeObject *type) +type_ready(PyTypeObject *type, int rerunbuiltin) { - _PyObject_ASSERT((PyObject *)type, - (type->tp_flags & Py_TPFLAGS_READYING) == 0); - type->tp_flags |= Py_TPFLAGS_READYING; + _PyObject_ASSERT((PyObject *)type, !is_readying(type)); + start_readying(type); if (type_ready_pre_checks(type) < 0) { goto error; @@ -7125,17 +7250,19 @@ type_ready(PyTypeObject *type) if (type_ready_mro(type) < 0) { goto error; } - if (type_ready_set_new(type) < 0) { + if (type_ready_set_new(type, rerunbuiltin) < 0) { goto error; } if (type_ready_fill_dict(type) < 0) { goto error; } - if (type_ready_inherit(type) < 0) { - goto error; - } - if (type_ready_preheader(type) < 0) { - goto error; + if (!rerunbuiltin) { + if (type_ready_inherit(type) < 0) { + goto error; + } + if (type_ready_preheader(type) < 0) { + goto error; + } } if (type_ready_set_hash(type) < 0) { goto error; @@ -7143,21 +7270,24 @@ type_ready(PyTypeObject *type) if (type_ready_add_subclasses(type) < 0) { goto error; } - if (type_ready_managed_dict(type) < 0) { - goto error; - } - if (type_ready_post_checks(type) < 0) { - goto error; + if (!rerunbuiltin) { + if (type_ready_managed_dict(type) < 0) { + goto error; + } + if (type_ready_post_checks(type) < 0) { + goto error; + } } /* All done -- set the ready flag */ - type->tp_flags = (type->tp_flags & ~Py_TPFLAGS_READYING) | Py_TPFLAGS_READY; + type->tp_flags = type->tp_flags | Py_TPFLAGS_READY; + stop_readying(type); assert(_PyType_CheckConsistency(type)); return 0; error: - type->tp_flags &= ~Py_TPFLAGS_READYING; + stop_readying(type); return -1; } @@ -7175,7 +7305,7 @@ PyType_Ready(PyTypeObject *type) type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE; } - return type_ready(type); + return type_ready(type, 0); } int @@ -7186,35 +7316,26 @@ _PyStaticType_InitBuiltin(PyInterpreterState *interp, PyTypeObject *self) assert(!(self->tp_flags & Py_TPFLAGS_MANAGED_DICT)); assert(!(self->tp_flags & Py_TPFLAGS_MANAGED_WEAKREF)); -#ifndef NDEBUG int ismain = _Py_IsMainInterpreter(interp); -#endif - if (self->tp_flags & Py_TPFLAGS_READY) { + if ((self->tp_flags & Py_TPFLAGS_READY) == 0) { + assert(ismain); + + self->tp_flags |= _Py_TPFLAGS_STATIC_BUILTIN; + self->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE; + + assert(NEXT_GLOBAL_VERSION_TAG <= _Py_MAX_GLOBAL_TYPE_VERSION_TAG); + self->tp_version_tag = NEXT_GLOBAL_VERSION_TAG++; + self->tp_flags |= Py_TPFLAGS_VALID_VERSION_TAG; + } + else { assert(!ismain); assert(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN); assert(self->tp_flags & Py_TPFLAGS_VALID_VERSION_TAG); - - static_builtin_state_init(interp, self); - - /* Per-interpreter tp_subclasses is done lazily. - Otherwise we would initialize it here. */ - - assert(_PyType_CheckConsistency(self)); - return 0; } - assert(ismain); - - self->tp_flags |= _Py_TPFLAGS_STATIC_BUILTIN; - self->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE; - - assert(NEXT_GLOBAL_VERSION_TAG <= _Py_MAX_GLOBAL_TYPE_VERSION_TAG); - self->tp_version_tag = NEXT_GLOBAL_VERSION_TAG++; - self->tp_flags |= Py_TPFLAGS_VALID_VERSION_TAG; - static_builtin_state_init(interp, self); - int res = type_ready(self); + int res = type_ready(self, !ismain); if (res < 0) { static_builtin_state_clear(interp, self); } -- cgit v0.12