From 01fa907aa8e7c475a76b407f35c635b26c9f47f8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 16 Nov 2022 09:54:28 -0700 Subject: [PATCH] gh-81057: Move contextvars-related Globals to _PyRuntimeState (gh-99400) This is part of the effort to consolidate global variables, to make them easier to manage (and make it easier to later move some of them to PyInterpreterState). https://github.com/python/cpython/issues/81057 --- Include/internal/pycore_context.h | 4 + Include/internal/pycore_global_objects.h | 9 ++ .../pycore_global_objects_fini_generated.h | 3 + Include/internal/pycore_hamt.h | 11 ++- Include/internal/pycore_runtime_init.h | 10 +++ Python/context.c | 38 +++----- Python/hamt.c | 86 +++++++------------ Tools/build/generate_global_objects.py | 3 + Tools/c-analyzer/cpython/globals-to-fix.tsv | 8 -- 9 files changed, 82 insertions(+), 90 deletions(-) diff --git a/Include/internal/pycore_context.h b/Include/internal/pycore_context.h index 1bf4e8f3ee5..52dfe3ef233 100644 --- a/Include/internal/pycore_context.h +++ b/Include/internal/pycore_context.h @@ -18,6 +18,10 @@ void _PyContext_Fini(PyInterpreterState *); /* other API */ +typedef struct { + PyObject_HEAD +} _PyContextTokenMissing; + #ifndef WITH_FREELISTS // without freelists # define PyContext_MAXFREELIST 0 diff --git a/Include/internal/pycore_global_objects.h b/Include/internal/pycore_global_objects.h index 5ad1f7d217f..80f6bb2c8a7 100644 --- a/Include/internal/pycore_global_objects.h +++ b/Include/internal/pycore_global_objects.h @@ -10,6 +10,8 @@ extern "C" { #include "pycore_gc.h" // PyGC_Head #include "pycore_global_strings.h" // struct _Py_global_strings +#include "pycore_hamt.h" // PyHamtNode_Bitmap +#include "pycore_context.h" // _PyContextTokenMissing #include "pycore_typeobject.h" // pytype_slotdef @@ -52,6 +54,10 @@ struct _Py_global_objects { _PyGC_Head_UNUSED _tuple_empty_gc_not_used; PyTupleObject tuple_empty; + + _PyGC_Head_UNUSED _hamt_bitmap_node_empty_gc_not_used; + PyHamtNode_Bitmap hamt_bitmap_node_empty; + _PyContextTokenMissing context_token_missing; } singletons; PyObject *interned; @@ -76,6 +82,9 @@ struct _Py_interp_cached_objects { struct _Py_interp_static_objects { struct { int _not_used; + // hamt_empty is here instead of global because of its weakreflist. + _PyGC_Head_UNUSED _hamt_empty_gc_not_used; + PyHamtObject hamt_empty; } singletons; }; diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 0b5833c19d6..5fd08f281f5 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -1475,6 +1475,9 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { /* non-generated */ _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(bytes_empty)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(tuple_empty)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(hamt_bitmap_node_empty)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_INTERP_SINGLETON(interp, hamt_empty)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(context_token_missing)); } #endif // Py_DEBUG /* End auto-generated code */ diff --git a/Include/internal/pycore_hamt.h b/Include/internal/pycore_hamt.h index 7a674b4ee4a..d8742c7cb63 100644 --- a/Include/internal/pycore_hamt.h +++ b/Include/internal/pycore_hamt.h @@ -28,10 +28,6 @@ extern PyTypeObject _PyHamtKeys_Type; extern PyTypeObject _PyHamtValues_Type; extern PyTypeObject _PyHamtItems_Type; -/* runtime lifecycle */ - -void _PyHamt_Fini(PyInterpreterState *); - /* other API */ @@ -53,6 +49,13 @@ typedef struct { } PyHamtObject; +typedef struct { + PyObject_VAR_HEAD + uint32_t b_bitmap; + PyObject *b_array[1]; +} PyHamtNode_Bitmap; + + /* A struct to hold the state of depth-first traverse of the tree. HAMT is an immutable collection. Iterators will hold a strong reference diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 37bc54ff96c..334928e58ac 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -75,6 +75,12 @@ extern "C" { .tuple_empty = { \ .ob_base = _PyVarObject_IMMORTAL_INIT(&PyTuple_Type, 0) \ }, \ + .hamt_bitmap_node_empty = { \ + .ob_base = _PyVarObject_IMMORTAL_INIT(&_PyHamt_BitmapNode_Type, 0) \ + }, \ + .context_token_missing = { \ + .ob_base = _PyObject_IMMORTAL_INIT(&_PyContextTokenMissing_Type), \ + }, \ }, \ }, \ ._main_interpreter = _PyInterpreterState_INIT, \ @@ -112,6 +118,10 @@ extern "C" { .static_objects = { \ .singletons = { \ ._not_used = 1, \ + .hamt_empty = { \ + .ob_base = _PyObject_IMMORTAL_INIT(&_PyHamt_Type), \ + .h_root = (PyHamtNode*)&_Py_SINGLETON(hamt_bitmap_node_empty), \ + }, \ }, \ }, \ ._initial_thread = _PyThreadState_INIT, \ diff --git a/Python/context.c b/Python/context.c index ee5a671ba26..5d385508405 100644 --- a/Python/context.c +++ b/Python/context.c @@ -1235,25 +1235,29 @@ token_new(PyContext *ctx, PyContextVar *var, PyObject *val) /////////////////////////// Token.MISSING -static PyObject *_token_missing; - - -typedef struct { - PyObject_HEAD -} PyContextTokenMissing; - - static PyObject * context_token_missing_tp_repr(PyObject *self) { return PyUnicode_FromString(""); } +static void +context_token_missing_tp_dealloc(_PyContextTokenMissing *Py_UNUSED(self)) +{ +#ifdef Py_DEBUG + /* The singleton is statically allocated. */ + _Py_FatalRefcountError("deallocating the token missing singleton"); +#else + return; +#endif +} + PyTypeObject _PyContextTokenMissing_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "Token.MISSING", - sizeof(PyContextTokenMissing), + sizeof(_PyContextTokenMissing), + .tp_dealloc = (destructor)context_token_missing_tp_dealloc, .tp_getattro = PyObject_GenericGetAttr, .tp_flags = Py_TPFLAGS_DEFAULT, .tp_repr = context_token_missing_tp_repr, @@ -1263,17 +1267,7 @@ PyTypeObject _PyContextTokenMissing_Type = { static PyObject * get_token_missing(void) { - if (_token_missing != NULL) { - return Py_NewRef(_token_missing); - } - - _token_missing = (PyObject *)PyObject_New( - PyContextTokenMissing, &_PyContextTokenMissing_Type); - if (_token_missing == NULL) { - return NULL; - } - - return Py_NewRef(_token_missing); + return Py_NewRef(&_Py_SINGLETON(context_token_missing)); } @@ -1298,15 +1292,11 @@ _PyContext_ClearFreeList(PyInterpreterState *interp) void _PyContext_Fini(PyInterpreterState *interp) { - if (_Py_IsMainInterpreter(interp)) { - Py_CLEAR(_token_missing); - } _PyContext_ClearFreeList(interp); #if defined(Py_DEBUG) && PyContext_MAXFREELIST > 0 struct _Py_context_state *state = &interp->context; state->numfree = -1; #endif - _PyHamt_Fini(interp); } diff --git a/Python/hamt.c b/Python/hamt.c index d94d0700213..4e61a1fc2b2 100644 --- a/Python/hamt.c +++ b/Python/hamt.c @@ -319,13 +319,6 @@ typedef struct { } PyHamtNode_Array; -typedef struct { - PyObject_VAR_HEAD - uint32_t b_bitmap; - PyObject *b_array[1]; -} PyHamtNode_Bitmap; - - typedef struct { PyObject_VAR_HEAD int32_t c_hash; @@ -333,10 +326,6 @@ typedef struct { } PyHamtNode_Collision; -static PyHamtNode_Bitmap *_empty_bitmap_node; -static PyHamtObject *_empty_hamt; - - static PyHamtObject * hamt_alloc(void); @@ -521,13 +510,16 @@ hamt_node_bitmap_new(Py_ssize_t size) PyHamtNode_Bitmap *node; Py_ssize_t i; + if (size == 0) { + /* Since bitmap nodes are immutable, we can cache the instance + for size=0 and reuse it whenever we need an empty bitmap node. + */ + return (PyHamtNode *)Py_NewRef(&_Py_SINGLETON(hamt_bitmap_node_empty)); + } + assert(size >= 0); assert(size % 2 == 0); - if (size == 0 && _empty_bitmap_node != NULL) { - return (PyHamtNode *)Py_NewRef(_empty_bitmap_node); - } - /* No freelist; allocate a new bitmap node */ node = PyObject_GC_NewVar( PyHamtNode_Bitmap, &_PyHamt_BitmapNode_Type, size); @@ -545,13 +537,6 @@ hamt_node_bitmap_new(Py_ssize_t size) _PyObject_GC_TRACK(node); - if (size == 0 && _empty_bitmap_node == NULL) { - /* Since bitmap nodes are immutable, we can cache the instance - for size=0 and reuse it whenever we need an empty bitmap node. - */ - _empty_bitmap_node = (PyHamtNode_Bitmap*)Py_NewRef(node); - } - return (PyHamtNode *)node; } @@ -1142,6 +1127,16 @@ hamt_node_bitmap_dealloc(PyHamtNode_Bitmap *self) Py_ssize_t len = Py_SIZE(self); Py_ssize_t i; + if (Py_SIZE(self) == 0) { + /* The empty node is statically allocated. */ + assert(self == &_Py_SINGLETON(hamt_bitmap_node_empty)); +#ifdef Py_DEBUG + _Py_FatalRefcountError("deallocating the empty hamt node bitmap singleton"); +#else + return; +#endif + } + PyObject_GC_UnTrack(self); Py_TRASHCAN_BEGIN(self, hamt_node_bitmap_dealloc) @@ -2431,33 +2426,15 @@ hamt_alloc(void) return o; } +#define _empty_hamt \ + (&_Py_INTERP_SINGLETON(_PyInterpreterState_Get(), hamt_empty)) + PyHamtObject * _PyHamt_New(void) { - if (_empty_hamt != NULL) { - /* HAMT is an immutable object so we can easily cache an - empty instance. */ - return (PyHamtObject*)Py_NewRef(_empty_hamt); - } - - PyHamtObject *o = hamt_alloc(); - if (o == NULL) { - return NULL; - } - - o->h_root = hamt_node_bitmap_new(0); - if (o->h_root == NULL) { - Py_DECREF(o); - return NULL; - } - - o->h_count = 0; - - if (_empty_hamt == NULL) { - _empty_hamt = (PyHamtObject*)Py_NewRef(o); - } - - return o; + /* HAMT is an immutable object so we can easily cache an + empty instance. */ + return (PyHamtObject*)Py_NewRef(_empty_hamt); } #ifdef Py_DEBUG @@ -2673,6 +2650,15 @@ hamt_tp_traverse(PyHamtObject *self, visitproc visit, void *arg) static void hamt_tp_dealloc(PyHamtObject *self) { + if (self == _empty_hamt) { + /* The empty one is statically allocated. */ +#ifdef Py_DEBUG + _Py_FatalRefcountError("deallocating the empty hamt singleton"); +#else + return; +#endif + } + PyObject_GC_UnTrack(self); if (self->h_weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*)self); @@ -2908,11 +2894,3 @@ PyTypeObject _PyHamt_CollisionNode_Type = { .tp_free = PyObject_GC_Del, .tp_hash = PyObject_HashNotImplemented, }; - - -void -_PyHamt_Fini(PyInterpreterState *interp) -{ - Py_CLEAR(_empty_hamt); - Py_CLEAR(_empty_bitmap_node); -} diff --git a/Tools/build/generate_global_objects.py b/Tools/build/generate_global_objects.py index b4243273ff4..9ceae89878c 100644 --- a/Tools/build/generate_global_objects.py +++ b/Tools/build/generate_global_objects.py @@ -127,6 +127,9 @@ NON_GENERATED_IMMORTAL_OBJECTS = [ # The generated ones come from generate_runtime_init(). '(PyObject *)&_Py_SINGLETON(bytes_empty)', '(PyObject *)&_Py_SINGLETON(tuple_empty)', + '(PyObject *)&_Py_SINGLETON(hamt_bitmap_node_empty)', + '(PyObject *)&_Py_INTERP_SINGLETON(interp, hamt_empty)', + '(PyObject *)&_Py_SINGLETON(context_token_missing)', ] diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index ee0c85bd425..87d8f637357 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -298,14 +298,6 @@ Objects/setobject.c - _dummy_struct - Objects/setobject.c - _PySet_Dummy - Objects/sliceobject.c - _Py_EllipsisObject - -#----------------------- -# other - -# initialized once -Python/context.c - _token_missing - -Python/hamt.c - _empty_bitmap_node - -Python/hamt.c - _empty_hamt - - ################################## # global non-objects to fix in core code