Sorry, something went wrong.
|
You can use this in deepfrozen modules to get this even faster see cpython/Tools/scripts/deepfreeze.py Line 140 in 74127b8 |
Sorry, something went wrong.
|
Overall: 0% faster compared to the main branch That's great news! I'm going to update PEP 683 with the outcome and some of the details. |
Sorry, something went wrong.
There was a problem hiding this comment.
Nice work! Here are some random review comments. Hopefully they're helpful. I decided to only review the final PR (with all optimizations). I skipped the .py files and a few others for now.
Sorry, something went wrong.
| immortalize_object(PyObject *obj, PyObject *Py_UNUSED(ignored)) | ||
| { | ||
| _Py_SetImmortal(obj); | ||
| /* Special case for PyCodeObjects since they don't have a tp_traverse */ |
There was a problem hiding this comment.
Various fields below are tuples and the individual items in the tuples should also become immortal, and for co_consts this should recurse down. Maybe whenever we make a tuple immortal we should immortalize all its items?
Sorry, something went wrong.
| Py_TYPE(FROM_GC(gc))->tp_traverse( | ||
| FROM_GC(gc), (visitproc)immortalize_object, NULL); |
There was a problem hiding this comment.
Will this loop find code objects contained inside other code objects? (I don't know what exactly is contained in permanent_generation.head.)
Sorry, something went wrong.
There was a problem hiding this comment.
code objects are not tracked by GC.
And most tuples are not tracked too.
So we need to find code and tuples via function objects, module global dict, class namespace dict, etc.
Sorry, something went wrong.
| goto error; | ||
| } | ||
| // Immortalize top level modules | ||
| if (tstate->recursion_limit - tstate->recursion_remaining == 1) { |
There was a problem hiding this comment.
Does this work? I put a printf here and it doesn't seem to be immortalizing most of the frozen modules:
(Most frozen modules are imported at a much higher recursion level, either 7, 13 or 18.)
Sorry, something went wrong.
| #define Py_SIZE(ob) Py_SIZE(_PyVarObject_CAST_CONST(ob)) | ||
|
|
||
| PyAPI_FUNC(PyObject *) _PyGC_ImmortalizeHeap(void); | ||
| PyAPI_FUNC(PyObject *) _PyGC_TransitiveImmortalize(PyObject *obj); |
There was a problem hiding this comment.
(This name is awkward, I'd expect some variation on "immortalize transitively".)
Sorry, something went wrong.
| if (weaklist != NULL) { \ | ||
| if (stdlib_list != NULL) { \ | ||
| PyObject *list = user_weaklist; \ | ||
| if (PySequence_Contains(stdlib_list, name)) { \ | ||
| list = stdlib_weaklist; \ | ||
| } \ |
There was a problem hiding this comment.
To be (hyper-)correct you probably need to check for list != NULL after this, since it might just be possible that stdlib_list is not NULL but stdlib_weaklist or user_weaklist is NULL.
Sorry, something went wrong.
| finalize_modules_clear_weaklist(PyThreadState *tstate, | ||
| PyInterpreterState *interp, |
There was a problem hiding this comment.
Isn't the interpreter state easily found from the thread state? So you'd only need a tstate arg.
Sorry, something went wrong.
| // detect those modules which have been held alive. | ||
| PyObject *weaklist = finalize_remove_modules(modules, verbose); | ||
| // This prepares two lists, the user defined list of modules as well | ||
| // as stdlib list of modules. The user modules will be destroyed first in |
There was a problem hiding this comment.
| // as stdlib list of modules. The user modules will be destroyed first in | |
| // as the stdlib list of modules. The stdlib modules will be destroyed after all user modules in |
(And probably reflow.)
Sorry, something went wrong.
| PyObject *key, *value; | ||
|
|
||
|
|
||
| /* First, clear only names starting with a single underscore */ |
There was a problem hiding this comment.
Stray comment?
Also, no phase deletes __builtins__, right?
Sorry, something went wrong.
| } | ||
|
|
||
| void | ||
| _PyModule_Clear(PyObject *m) |
There was a problem hiding this comment.
IIUC this is no longer used. Is that right?
Sorry, something went wrong.
| _Py_RefTotal++; | ||
| #endif | ||
| Py_SET_REFCNT(op, 1); | ||
| /* Do not use Py_SET_REFCNT to skip the Immortal Instance check. This |
There was a problem hiding this comment.
| /* Do not use Py_SET_REFCNT to skip the Immortal Instance check. This | |
| /* Do not use Py_SET_REFCNT -- it skips the Immortal Instance check. This |
Sorry, something went wrong.
| } | ||
| if (from_prev) { | ||
| _PyGCHead_SET_PREV(from_next, from_prev); | ||
| } |
There was a problem hiding this comment.
Why is this change needed at all?
And please use 4-space indent.
Sorry, something went wrong.
| } | ||
|
|
||
| PyObject * | ||
| _PyGC_ImmortalizeHeap(void) { |
There was a problem hiding this comment.
I don't like this name. This function don't relating to "Heap" at all.
And please move the { to next line.
Sorry, something went wrong.
| Py_TYPE(FROM_GC(gc))->tp_traverse( | ||
| FROM_GC(gc), (visitproc)immortalize_object, NULL); |
There was a problem hiding this comment.
code objects are not tracked by GC.
And most tuples are not tracked too.
So we need to find code and tuples via function objects, module global dict, class namespace dict, etc.
Sorry, something went wrong.
This is an optimization on top of PR19474.
It combines PR31488, PR31489, and PR31490 into a single change to measure the combined performance benefits.
These results do not change too much from what was already achieved independently by these optimizations (as some of the immortalized instances start overlaping with each other). That being said, performance will keep scaling as the application scales as well. The current microbenchmarks do not measure applications that contain hundreds of imports or thousands of interned strings. Nonetheless, it is still worthwhile to consider all of these improvements in conjunction when thinking about larger scale applications.
Benchmark Results
Overall: 0% faster compared to the main branch and the highest number of non-stat significant benchmarks (18)
pyperformance resultshttps://bugs.python.org/issue40255