From 15d4c9fabce67b8a1b5bd9dec9612014ec18291a Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 8 Sep 2023 10:34:40 +0100 Subject: [PATCH] GH-108716: Turn off deep-freezing of code objects. (GH-108722) --- Include/cpython/import.h | 1 - Include/internal/pycore_code.h | 2 - Include/internal/pycore_interp.h | 1 + Lib/test/test_ctypes/test_values.py | 1 - Makefile.pre.in | 1 - ...-08-28-03-38-28.gh-issue-108716.HJBPwt.rst | 2 + Objects/codeobject.c | 7 ++- Objects/funcobject.c | 9 +-- Programs/_bootstrap_python.c | 2 - Programs/_freeze_module.c | 2 - Python/frozen.c | 58 +++++++++---------- Python/import.c | 13 +---- Python/pylifecycle.c | 6 -- Python/pystate.c | 1 + Tools/build/freeze_modules.py | 30 +++------- 15 files changed, 50 insertions(+), 86 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-08-28-03-38-28.gh-issue-108716.HJBPwt.rst diff --git a/Include/cpython/import.h b/Include/cpython/import.h index cdfdd15bfa4..7daf0b84fcf 100644 --- a/Include/cpython/import.h +++ b/Include/cpython/import.h @@ -17,7 +17,6 @@ struct _frozen { const unsigned char *code; int size; int is_package; - PyObject *(*get_code)(void); }; /* Embedding apps may change this pointer to point to their favorite diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index b3f480c7204..257b0583eda 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -465,8 +465,6 @@ adaptive_counter_backoff(uint16_t counter) { return adaptive_counter_bits(value, backoff); } -extern uint32_t _Py_next_func_version; - /* Comparison bit masks. */ diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index e0b7a325929..ba5764e943e 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -186,6 +186,7 @@ struct _is { _PyOptimizerObject *optimizer; uint16_t optimizer_resume_threshold; uint16_t optimizer_backedge_threshold; + uint32_t next_func_version; _Py_GlobalMonitors monitors; bool f_opcode_trace_set; diff --git a/Lib/test/test_ctypes/test_values.py b/Lib/test/test_ctypes/test_values.py index 9f8b69409cb..d0b4803dff8 100644 --- a/Lib/test/test_ctypes/test_values.py +++ b/Lib/test/test_ctypes/test_values.py @@ -58,7 +58,6 @@ class PythonValuesTestCase(unittest.TestCase): ("code", POINTER(c_ubyte)), ("size", c_int), ("is_package", c_int), - ("get_code", POINTER(c_ubyte)), # Function ptr ] FrozenTable = POINTER(struct_frozen) diff --git a/Makefile.pre.in b/Makefile.pre.in index aa3eaabc755..19a80299783 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -505,7 +505,6 @@ LIBRARY_OBJS_OMIT_FROZEN= \ LIBRARY_OBJS= \ $(LIBRARY_OBJS_OMIT_FROZEN) \ - $(DEEPFREEZE_OBJS) \ Modules/getpath.o \ Python/frozen.o diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-28-03-38-28.gh-issue-108716.HJBPwt.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-28-03-38-28.gh-issue-108716.HJBPwt.rst new file mode 100644 index 00000000000..f63eb8689d6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-28-03-38-28.gh-issue-108716.HJBPwt.rst @@ -0,0 +1,2 @@ +Turn off deep-freezing of code objects. Modules are still frozen, so that a +file system search is not needed for common modules. diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 1443027ff29..d00bd0422f0 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -427,9 +427,10 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con) co->co_framesize = nlocalsplus + con->stacksize + FRAME_SPECIALS_SIZE; co->co_ncellvars = ncellvars; co->co_nfreevars = nfreevars; - co->co_version = _Py_next_func_version; - if (_Py_next_func_version != 0) { - _Py_next_func_version++; + PyInterpreterState *interp = _PyInterpreterState_GET(); + co->co_version = interp->next_func_version; + if (interp->next_func_version != 0) { + interp->next_func_version++; } co->_co_monitoring = NULL; co->_co_instrumentation_version = 0; diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 648b660859c..231a9c141d0 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -3,7 +3,6 @@ #include "Python.h" #include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals() -#include "pycore_code.h" // _Py_next_func_version #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_pyerrors.h" // _PyErr_Occurred() @@ -252,11 +251,9 @@ When the function version is 0, the `CALL` bytecode is not specialized. Code object versions -------------------- -So where to code objects get their `co_version`? There is a single -static global counter, `_Py_next_func_version`. This is initialized in -the generated (!) file `Python/deepfreeze/deepfreeze.c`, to 1 plus the -number of deep-frozen function objects in that file. -(In `_bootstrap_python.c` and `freeze_module.c` it is initialized to 1.) +So where to code objects get their `co_version`? +There is a per-interpreter counter, `next_func_version`. +This is initialized to 1 when the interpreter is created. Code objects get a new `co_version` allocated from this counter upon creation. Since code objects are nominally immutable, `co_version` can diff --git a/Programs/_bootstrap_python.c b/Programs/_bootstrap_python.c index 6c388fc7033..34f79191b4e 100644 --- a/Programs/_bootstrap_python.c +++ b/Programs/_bootstrap_python.c @@ -15,8 +15,6 @@ #include "Python/frozen_modules/zipimport.h" /* End includes */ -uint32_t _Py_next_func_version = 1; - /* Empty initializer for deepfrozen modules */ int _Py_Deepfreeze_Init(void) { diff --git a/Programs/_freeze_module.c b/Programs/_freeze_module.c index f6c46fa629e..3de6c6816c1 100644 --- a/Programs/_freeze_module.c +++ b/Programs/_freeze_module.c @@ -22,8 +22,6 @@ # include #endif -uint32_t _Py_next_func_version = 1; - /* Empty initializer for deepfrozen modules */ int _Py_Deepfreeze_Init(void) { diff --git a/Python/frozen.c b/Python/frozen.c index 6b977710e6e..0fb38a11902 100644 --- a/Python/frozen.c +++ b/Python/frozen.c @@ -101,46 +101,46 @@ extern PyObject *_Py_get_frozen_only_toplevel(void); /* End extern declarations */ static const struct _frozen bootstrap_modules[] = { - {"_frozen_importlib", _Py_M__importlib__bootstrap, (int)sizeof(_Py_M__importlib__bootstrap), false, GET_CODE(importlib__bootstrap)}, - {"_frozen_importlib_external", _Py_M__importlib__bootstrap_external, (int)sizeof(_Py_M__importlib__bootstrap_external), false, GET_CODE(importlib__bootstrap_external)}, - {"zipimport", _Py_M__zipimport, (int)sizeof(_Py_M__zipimport), false, GET_CODE(zipimport)}, + {"_frozen_importlib", _Py_M__importlib__bootstrap, (int)sizeof(_Py_M__importlib__bootstrap), false}, + {"_frozen_importlib_external", _Py_M__importlib__bootstrap_external, (int)sizeof(_Py_M__importlib__bootstrap_external), false}, + {"zipimport", _Py_M__zipimport, (int)sizeof(_Py_M__zipimport), false}, {0, 0, 0} /* bootstrap sentinel */ }; static const struct _frozen stdlib_modules[] = { /* stdlib - startup, without site (python -S) */ - {"abc", _Py_M__abc, (int)sizeof(_Py_M__abc), false, GET_CODE(abc)}, - {"codecs", _Py_M__codecs, (int)sizeof(_Py_M__codecs), false, GET_CODE(codecs)}, - {"io", _Py_M__io, (int)sizeof(_Py_M__io), false, GET_CODE(io)}, + {"abc", _Py_M__abc, (int)sizeof(_Py_M__abc), false}, + {"codecs", _Py_M__codecs, (int)sizeof(_Py_M__codecs), false}, + {"io", _Py_M__io, (int)sizeof(_Py_M__io), false}, /* stdlib - startup, with site */ - {"_collections_abc", _Py_M___collections_abc, (int)sizeof(_Py_M___collections_abc), false, GET_CODE(_collections_abc)}, - {"_sitebuiltins", _Py_M___sitebuiltins, (int)sizeof(_Py_M___sitebuiltins), false, GET_CODE(_sitebuiltins)}, - {"genericpath", _Py_M__genericpath, (int)sizeof(_Py_M__genericpath), false, GET_CODE(genericpath)}, - {"ntpath", _Py_M__ntpath, (int)sizeof(_Py_M__ntpath), false, GET_CODE(ntpath)}, - {"posixpath", _Py_M__posixpath, (int)sizeof(_Py_M__posixpath), false, GET_CODE(posixpath)}, - {"os.path", _Py_M__posixpath, (int)sizeof(_Py_M__posixpath), false, GET_CODE(posixpath)}, - {"os", _Py_M__os, (int)sizeof(_Py_M__os), false, GET_CODE(os)}, - {"site", _Py_M__site, (int)sizeof(_Py_M__site), false, GET_CODE(site)}, - {"stat", _Py_M__stat, (int)sizeof(_Py_M__stat), false, GET_CODE(stat)}, + {"_collections_abc", _Py_M___collections_abc, (int)sizeof(_Py_M___collections_abc), false}, + {"_sitebuiltins", _Py_M___sitebuiltins, (int)sizeof(_Py_M___sitebuiltins), false}, + {"genericpath", _Py_M__genericpath, (int)sizeof(_Py_M__genericpath), false}, + {"ntpath", _Py_M__ntpath, (int)sizeof(_Py_M__ntpath), false}, + {"posixpath", _Py_M__posixpath, (int)sizeof(_Py_M__posixpath), false}, + {"os.path", _Py_M__posixpath, (int)sizeof(_Py_M__posixpath), false}, + {"os", _Py_M__os, (int)sizeof(_Py_M__os), false}, + {"site", _Py_M__site, (int)sizeof(_Py_M__site), false}, + {"stat", _Py_M__stat, (int)sizeof(_Py_M__stat), false}, /* runpy - run module with -m */ - {"importlib.util", _Py_M__importlib_util, (int)sizeof(_Py_M__importlib_util), false, GET_CODE(importlib_util)}, - {"importlib.machinery", _Py_M__importlib_machinery, (int)sizeof(_Py_M__importlib_machinery), false, GET_CODE(importlib_machinery)}, - {"runpy", _Py_M__runpy, (int)sizeof(_Py_M__runpy), false, GET_CODE(runpy)}, + {"importlib.util", _Py_M__importlib_util, (int)sizeof(_Py_M__importlib_util), false}, + {"importlib.machinery", _Py_M__importlib_machinery, (int)sizeof(_Py_M__importlib_machinery), false}, + {"runpy", _Py_M__runpy, (int)sizeof(_Py_M__runpy), false}, {0, 0, 0} /* stdlib sentinel */ }; static const struct _frozen test_modules[] = { - {"__hello__", _Py_M____hello__, (int)sizeof(_Py_M____hello__), false, GET_CODE(__hello__)}, - {"__hello_alias__", _Py_M____hello__, (int)sizeof(_Py_M____hello__), false, GET_CODE(__hello__)}, - {"__phello_alias__", _Py_M____hello__, (int)sizeof(_Py_M____hello__), true, GET_CODE(__hello__)}, - {"__phello_alias__.spam", _Py_M____hello__, (int)sizeof(_Py_M____hello__), false, GET_CODE(__hello__)}, - {"__phello__", _Py_M____phello__, (int)sizeof(_Py_M____phello__), true, GET_CODE(__phello__)}, - {"__phello__.__init__", _Py_M____phello__, (int)sizeof(_Py_M____phello__), false, GET_CODE(__phello__)}, - {"__phello__.ham", _Py_M____phello___ham, (int)sizeof(_Py_M____phello___ham), true, GET_CODE(__phello___ham)}, - {"__phello__.ham.__init__", _Py_M____phello___ham, (int)sizeof(_Py_M____phello___ham), false, GET_CODE(__phello___ham)}, - {"__phello__.ham.eggs", _Py_M____phello___ham_eggs, (int)sizeof(_Py_M____phello___ham_eggs), false, GET_CODE(__phello___ham_eggs)}, - {"__phello__.spam", _Py_M____phello___spam, (int)sizeof(_Py_M____phello___spam), false, GET_CODE(__phello___spam)}, - {"__hello_only__", _Py_M__frozen_only, (int)sizeof(_Py_M__frozen_only), false, GET_CODE(frozen_only)}, + {"__hello__", _Py_M____hello__, (int)sizeof(_Py_M____hello__), false}, + {"__hello_alias__", _Py_M____hello__, (int)sizeof(_Py_M____hello__), false}, + {"__phello_alias__", _Py_M____hello__, (int)sizeof(_Py_M____hello__), true}, + {"__phello_alias__.spam", _Py_M____hello__, (int)sizeof(_Py_M____hello__), false}, + {"__phello__", _Py_M____phello__, (int)sizeof(_Py_M____phello__), true}, + {"__phello__.__init__", _Py_M____phello__, (int)sizeof(_Py_M____phello__), false}, + {"__phello__.ham", _Py_M____phello___ham, (int)sizeof(_Py_M____phello___ham), true}, + {"__phello__.ham.__init__", _Py_M____phello___ham, (int)sizeof(_Py_M____phello___ham), false}, + {"__phello__.ham.eggs", _Py_M____phello___ham_eggs, (int)sizeof(_Py_M____phello___ham_eggs), false}, + {"__phello__.spam", _Py_M____phello___spam, (int)sizeof(_Py_M____phello___spam), false}, + {"__hello_only__", _Py_M__frozen_only, (int)sizeof(_Py_M__frozen_only), false}, {0, 0, 0} /* test sentinel */ }; const struct _frozen *_PyImport_FrozenBootstrap = bootstrap_modules; diff --git a/Python/import.c b/Python/import.c index 48090d05240..126eb5e162a 100644 --- a/Python/import.c +++ b/Python/import.c @@ -2006,7 +2006,6 @@ look_up_frozen(const char *name) struct frozen_info { PyObject *nameobj; const char *data; - PyObject *(*get_code)(void); Py_ssize_t size; bool is_package; bool is_alias; @@ -2040,7 +2039,6 @@ find_frozen(PyObject *nameobj, struct frozen_info *info) if (info != NULL) { info->nameobj = nameobj; // borrowed info->data = (const char *)p->code; - info->get_code = p->get_code; info->size = p->size; info->is_package = p->is_package; if (p->size < 0) { @@ -2052,10 +2050,6 @@ find_frozen(PyObject *nameobj, struct frozen_info *info) info->is_alias = resolve_module_alias(name, _PyImport_FrozenAliases, &info->origname); } - if (p->code == NULL && p->size == 0 && p->get_code != NULL) { - /* It is only deepfrozen. */ - return FROZEN_OKAY; - } if (p->code == NULL) { /* It is frozen but marked as un-importable. */ return FROZEN_EXCLUDED; @@ -2070,11 +2064,6 @@ find_frozen(PyObject *nameobj, struct frozen_info *info) static PyObject * unmarshal_frozen_code(PyInterpreterState *interp, struct frozen_info *info) { - if (info->get_code && _Py_IsMainInterpreter(interp)) { - PyObject *code = info->get_code(); - assert(code != NULL); - return code; - } PyObject *co = PyMarshal_ReadObjectFromString(info->data, info->size); if (co == NULL) { /* Does not contain executable code. */ @@ -3567,7 +3556,7 @@ _imp_get_frozen_object_impl(PyObject *module, PyObject *name, if (info.nameobj == NULL) { info.nameobj = name; } - if (info.size == 0 && info.get_code == NULL) { + if (info.size == 0) { /* Does not contain executable code. */ set_frozen_error(FROZEN_INVALID, name); return NULL; diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 92eef6d5071..48000153854 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -832,11 +832,6 @@ pycore_interp_init(PyThreadState *tstate) if (_PyStatus_EXCEPTION(status)) { return status; } - // Intern strings in deep-frozen modules first so that others - // can use it instead of creating a heap allocated string. - if (_Py_Deepfreeze_Init() < 0) { - return _PyStatus_ERR("failed to initialize deep-frozen modules"); - } status = pycore_init_types(interp); if (_PyStatus_EXCEPTION(status)) { @@ -1743,7 +1738,6 @@ finalize_interp_clear(PyThreadState *tstate) _Py_HashRandomization_Fini(); _PyArg_Fini(); _Py_ClearFileSystemEncoding(); - _Py_Deepfreeze_Fini(); _PyPerfTrampoline_Fini(); } diff --git a/Python/pystate.c b/Python/pystate.c index b2b9b9f8776..ed14f82688f 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -697,6 +697,7 @@ init_interpreter(PyInterpreterState *interp, interp->optimizer = &_PyOptimizer_Default; interp->optimizer_backedge_threshold = _PyOptimizer_Default.backedge_threshold; interp->optimizer_resume_threshold = _PyOptimizer_Default.backedge_threshold; + interp->next_func_version = 1; if (interp != &runtime->_main_interpreter) { /* Fix the self-referential, statically initialized fields. */ interp->dtoa = (struct _dtoa_state)_dtoa_state_INIT(interp); diff --git a/Tools/build/freeze_modules.py b/Tools/build/freeze_modules.py index 12200979fa4..a07f4d9786e 100644 --- a/Tools/build/freeze_modules.py +++ b/Tools/build/freeze_modules.py @@ -467,15 +467,14 @@ def replace_block(lines, start_marker, end_marker, replacements, file): return lines[:start_pos + 1] + replacements + lines[end_pos:] -def regen_frozen(modules, frozen_modules: bool): +def regen_frozen(modules): headerlines = [] parentdir = os.path.dirname(FROZEN_FILE) - if frozen_modules: - for src in _iter_sources(modules): - # Adding a comment to separate sections here doesn't add much, - # so we don't. - header = relpath_for_posix_display(src.frozenfile, parentdir) - headerlines.append(f'#include "{header}"') + for src in _iter_sources(modules): + # Adding a comment to separate sections here doesn't add much, + # so we don't. + header = relpath_for_posix_display(src.frozenfile, parentdir) + headerlines.append(f'#include "{header}"') externlines = [] bootstraplines = [] @@ -504,14 +503,9 @@ def regen_frozen(modules, frozen_modules: bool): get_code_name = "_Py_get_%s_toplevel" % code_name externlines.append("extern PyObject *%s(void);" % get_code_name) - symbol = mod.symbol pkg = 'true' if mod.ispkg else 'false' - if not frozen_modules: - line = ('{"%s", NULL, 0, %s, GET_CODE(%s)},' - ) % (mod.name, pkg, code_name) - else: - line = ('{"%s", %s, (int)sizeof(%s), %s, GET_CODE(%s)},' - ) % (mod.name, symbol, symbol, pkg, code_name) + size = f"(int)sizeof({mod.symbol})" + line = f'{{"{mod.name}", {mod.symbol}, {size}, {pkg}}},' lines.append(line) if mod.isalias: @@ -718,20 +712,14 @@ def regen_pcbuild(modules): ####################################### # the script -parser = argparse.ArgumentParser() -parser.add_argument("--frozen-modules", action="store_true", - help="Use both frozen and deepfrozen modules. (default: uses only deepfrozen modules)") - def main(): - args = parser.parse_args() - frozen_modules: bool = args.frozen_modules # Expand the raw specs, preserving order. modules = list(parse_frozen_specs()) # Regen build-related files. regen_makefile(modules) regen_pcbuild(modules) - regen_frozen(modules, frozen_modules) + regen_frozen(modules) if __name__ == '__main__':