bpo-46712: Share global string identifiers in deepfreeze#31261
Conversation
kumaraditya303force-pushed the global-strings branch from a173ee9 to 3bd4e2aCompare
ericsnowcurrently commented Feb 14, 2022
Extrapolating the list of global strings dynamically is a good idea. It would be best to do it in a separate PR. I've done so in gh-31346. |
Instead of manually enumerating the global strings in generate_global_objects.py, we extrapolate the list from usage of _Py_ID() and _Py_STR() in the source files. This is partly inspired by gh-31261. https://bugs.python.org/issue46541
gvanrossum commented Feb 20, 2022
@kumaraditya303 Could you fix the conflicts? |
kumaraditya303force-pushed the global-strings branch from 470037f to e959ed6Compare
kumaraditya303 commented Feb 24, 2022
Fixed |
kumaraditya303 commented Feb 25, 2022
@gvanrossum This is ready for review. |
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
LGTM.
I looked at some of the generated output and this feels like it's a good improvement.
Uh oh!
There was an error while loading. Please reload this page.
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
Thanks. Waiting for the tests to run once more.
(Also, please go fix the interning error handling, even if you disagree.)
kumaraditya303 commented Feb 25, 2022
Okay, will create a PR tomorrow. |
gvanrossum commented Feb 25, 2022
Thanks Kumar! It's nice to see us regain some of the space costs of deep-freezing modules. |
Where appropriate, deepfreeze.c now uses `&_Py_ID(blah)` references instead of locally defining constants. This saves some space.
5 participants
Since bpo-46541, the global strings are statically allocated so they can now be referenced by deep-frozen modules just like any other singleton. Sharing identifiers with deepfreeze will reduce the duplicated strings hence it would save space.
See faster-cpython/ideas#218
See faster-cpython/ideas#230
https://bugs.python.org/issue46712