gh-140550: Use a bool for the Py_mod_gil value (GH-141519)

This needs a single bit, but was stored as a void* in the module
struct. This didn't matter due to packing, but now that there's
another bool in the struct, we can save a bit of memory by
making md_gil a bool.

Variables that changed type are renamed, to detect conflicts.

diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h
index 6eef6eaa5df84444bc9bd9263c050868428238aa..9a62daf6621ca2955bb3a89de70e0e844afb824c 100644 (file)
--- a/Include/internal/pycore_moduleobject.h
+++ b/Include/internal/pycore_moduleobject.h
@@ -30,7 +30,7 @@ typedef struct {
     PyObject *md_name;
     bool md_token_is_def;  /* if true, `md_token` is the PyModuleDef */
 #ifdef Py_GIL_DISABLED
-    void *md_gil;
+    bool md_requires_gil;
 #endif
     Py_ssize_t md_state_size;
     traverseproc md_state_traverse;

diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index 798f58737b1bf67656d12a37671ff47c707661a7..2f169c1165df05aed7488ee9440fba1a9a1c8691 100644 (file)
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -1725,7 +1725,7 @@ class SizeofTest(unittest.TestCase):
         check(int(PyLong_BASE**2), vsize('') + 3*self.longdigit)
         # module
         if support.Py_GIL_DISABLED:
-            md_gil = 'P'
+            md_gil = '?'
         else:
             md_gil = ''
         check(unittest, size('PPPP?' + md_gil + 'NPPPPP'))

diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index 9dee03bdb5ee551227e3d89d1b0805cda149f382..6c1c5f5eb89c0c71af6a775e60af658d846d9b14 100644 (file)
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -178,7 +178,7 @@ new_module_notrack(PyTypeObject *mt)
     m->md_name = NULL;
     m->md_token_is_def = false;
 #ifdef Py_GIL_DISABLED
-    m->md_gil = Py_MOD_GIL_USED;
+    m->md_requires_gil = true;
 #endif
     m->md_state_size = 0;
     m->md_state_traverse = NULL;
@@ -361,7 +361,7 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version)
     m->md_token_is_def = true;
     module_copy_members_from_deflike(m, module);
 #ifdef Py_GIL_DISABLED
-    m->md_gil = Py_MOD_GIL_USED;
+    m->md_requires_gil = true;
 #endif
     return (PyObject*)m;
 }
@@ -380,7 +380,7 @@ module_from_def_and_spec(
     int has_multiple_interpreters_slot = 0;
     void *multiple_interpreters = (void *)0;
     int has_gil_slot = 0;
-    void *gil_slot = Py_MOD_GIL_USED;
+    bool requires_gil = true;
     int has_execution_slots = 0;
     const char *name;
     int ret;
@@ -474,7 +474,7 @@ module_from_def_and_spec(
                        name);
                     goto error;
                 }
-                gil_slot = cur_slot->value;
+                requires_gil = (cur_slot->value != Py_MOD_GIL_NOT_USED);
                 has_gil_slot = 1;
                 break;
             case Py_mod_abi:
@@ -581,9 +581,9 @@ module_from_def_and_spec(
             mod->md_token = token;
         }
 #ifdef Py_GIL_DISABLED
-        mod->md_gil = gil_slot;
+        mod->md_requires_gil = requires_gil;
 #else
-        (void)gil_slot;
+        (void)requires_gil;
 #endif
         mod->md_exec = m_exec;
     } else {
@@ -664,11 +664,12 @@ PyModule_FromSlotsAndSpec(const PyModuleDef_Slot *slots, PyObject *spec)
 int
 PyUnstable_Module_SetGIL(PyObject *module, void *gil)
 {
+    bool requires_gil = (gil != Py_MOD_GIL_NOT_USED);
     if (!PyModule_Check(module)) {
         PyErr_BadInternalCall();
         return -1;
     }
-    ((PyModuleObject *)module)->md_gil = gil;
+    ((PyModuleObject *)module)->md_requires_gil = requires_gil;
     return 0;
 }
 #endif

diff --git a/Python/import.c b/Python/import.c
index 2afa7c15e6a8dc08bdc2e8ec09549e8cba1ec792..b05b40448d02acd4fb37a4d2970f01f0632218bb 100644 (file)
--- a/Python/import.c
+++ b/Python/import.c
@@ -1017,9 +1017,10 @@ struct extensions_cache_value {
     _Py_ext_module_origin origin;
 
 #ifdef Py_GIL_DISABLED
-    /* The module's md_gil slot, for legacy modules that are reinitialized from
-       m_dict rather than calling their initialization function again. */
-    void *md_gil;
+    /* The module's md_requires_gil member, for legacy modules that are
+     * reinitialized from m_dict rather than calling their initialization
+     * function again. */
+    bool md_requires_gil;
 #endif
 };
 
@@ -1350,7 +1351,7 @@ static struct extensions_cache_value *
 _extensions_cache_set(PyObject *path, PyObject *name,
                       PyModuleDef *def, PyModInitFunction m_init,
                       Py_ssize_t m_index, PyObject *m_dict,
-                      _Py_ext_module_origin origin, void *md_gil)
+                      _Py_ext_module_origin origin, bool requires_gil)
 {
     struct extensions_cache_value *value = NULL;
     void *key = NULL;
@@ -1405,11 +1406,11 @@ _extensions_cache_set(PyObject *path, PyObject *name,
         /* m_dict is set by set_cached_m_dict(). */
         .origin=origin,
 #ifdef Py_GIL_DISABLED
-        .md_gil=md_gil,
+        .md_requires_gil=requires_gil,
 #endif
     };
 #ifndef Py_GIL_DISABLED
-    (void)md_gil;
+    (void)requires_gil;
 #endif
     if (init_cached_m_dict(newvalue, m_dict) < 0) {
         goto finally;
@@ -1547,7 +1548,8 @@ _PyImport_CheckGILForModule(PyObject* module, PyObject *module_name)
     }
 
     if (!PyModule_Check(module) ||
-        ((PyModuleObject *)module)->md_gil == Py_MOD_GIL_USED) {
+        ((PyModuleObject *)module)->md_requires_gil)
+    {
         if (_PyEval_EnableGILPermanent(tstate)) {
             int warn_result = PyErr_WarnFormat(
                 PyExc_RuntimeWarning,
@@ -1725,7 +1727,7 @@ struct singlephase_global_update {
     Py_ssize_t m_index;
     PyObject *m_dict;
     _Py_ext_module_origin origin;
-    void *md_gil;
+    bool md_requires_gil;
 };
 
 static struct extensions_cache_value *
@@ -1784,7 +1786,7 @@ update_global_state_for_extension(PyThreadState *tstate,
 #endif
         cached = _extensions_cache_set(
                 path, name, def, m_init, singlephase->m_index, m_dict,
-                singlephase->origin, singlephase->md_gil);
+                singlephase->origin, singlephase->md_requires_gil);
         if (cached == NULL) {
             // XXX Ignore this error?  Doing so would effectively
             // mark the module as not loadable.
@@ -1873,7 +1875,7 @@ reload_singlephase_extension(PyThreadState *tstate,
         if (def->m_base.m_copy != NULL) {
             // For non-core modules, fetch the GIL slot that was stored by
             // import_run_extension().
-            ((PyModuleObject *)mod)->md_gil = cached->md_gil;
+            ((PyModuleObject *)mod)->md_requires_gil = cached->md_requires_gil;
         }
 #endif
         /* We can't set mod->md_def if it's missing,
@@ -2128,7 +2130,7 @@ import_run_extension(PyThreadState *tstate, PyModInitFunction p0,
                 .m_index=def->m_base.m_index,
                 .origin=info->origin,
 #ifdef Py_GIL_DISABLED
-                .md_gil=((PyModuleObject *)mod)->md_gil,
+                .md_requires_gil=((PyModuleObject *)mod)->md_requires_gil,
 #endif
             };
             // gh-88216: Extensions and def->m_base.m_copy can be updated
@@ -2323,7 +2325,7 @@ _PyImport_FixupBuiltin(PyThreadState *tstate, PyObject *mod, const char *name,
             .origin=_Py_ext_module_origin_CORE,
 #ifdef Py_GIL_DISABLED
             /* Unused when m_dict == NULL. */
-            .md_gil=NULL,
+            .md_requires_gil=false,
 #endif
         };
         cached = update_global_state_for_extension(