GH-108716: Turn off deep-freezing of code objects. (GH-108722)

markshannon · web-flow · commit 15d4c9fabce6 · 2023-09-08T10:34:40.000+01:00
diff --git 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
@@ -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
@@ -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
@@ -58,7 +58,6 @@ class struct_frozen(Structure):
                         ("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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -22,8 +22,6 @@
 #  include <unistd.h>
 #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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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__':

Original file line number	Diff line number	Diff line change
`@@ -465,8 +465,6 @@ adaptive_counter_backoff(uint16_t counter) {`
`465`	`465`	`return adaptive_counter_bits(value, backoff);`
`466`	`466`	`}`
`467`	`467`
`468`		`-extern uint32_t _Py_next_func_version;`
`469`		`-`
`470`	`468`
`471`	`469`	`/* Comparison bit masks. */`
`472`	`470`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,6 @@ class struct_frozen(Structure):`
`58`	`58`	`("code", POINTER(c_ubyte)),`
`59`	`59`	`("size", c_int),`
`60`	`60`	`("is_package", c_int),`
`61`		`- ("get_code", POINTER(c_ubyte)), # Function ptr`
`62`	`61`	`]`
`63`	`62`	`FrozenTable = POINTER(struct_frozen)`
`64`	`63`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Turn off deep-freezing of code objects. Modules are still frozen, so that a`
	`2`	`+file system search is not needed for common modules.`
Original file line number	Diff line number	Diff line change
`@@ -15,8 +15,6 @@`
`15`	`15`	`#include "Python/frozen_modules/zipimport.h"`
`16`	`16`	`/* End includes */`
`17`	`17`
`18`		`-uint32_t _Py_next_func_version = 1;`
`19`		`-`
`20`	`18`	`/* Empty initializer for deepfrozen modules */`
`21`	`19`	`int _Py_Deepfreeze_Init(void)`
`22`	`20`	`{`
Original file line number	Diff line number	Diff line change
`@@ -22,8 +22,6 @@`
`22`	`22`	`# include <unistd.h>`
`23`	`23`	`#endif`
`24`	`24`
`25`		`-uint32_t _Py_next_func_version = 1;`
`26`		`-`
`27`	`25`	`/* Empty initializer for deepfrozen modules */`
`28`	`26`	`int _Py_Deepfreeze_Init(void)`
`29`	`27`	`{`