-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
GH-96458: Statically initialize utf8 representation of static strings #96481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
kumaraditya303
commented
Sep 1, 2022
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: _PyStaticUnicode_Dealloc should not exist. #96458
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_PyUnicode_LATIN1_INIT
is wrong name. You can statically initialize utf8 representation only in ASCII strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like @ericsnowcurrently to review this too.
Are you generating utf8 strings separately from the ascii string, or do they share the same data? |
The utf8 cache strings are only ever created and used for non-ASCII strings (i.e. at least one character is 128 or larger). The unicode string object is smart enough to know that for pure ASCII strings the utf8 is identical so it doesn't bother creating or using the utf8 cache value. This PR only generates utf8 data for exactly 6 strings in deepfreeze.c. I'm sure those are the ones with at least non-ASCII character. In order to see a leak one would have to request the utf8 encoding of at least one of those 6 strings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What do you mean? Keep in mind that the macro is used exclusively to statically initialize latin-1 strings. |
Indeed -- in fact the macro is only used in pycore_runtime_init_generated.h (a generated file :-) to initialize the |
Sorry, now I see that |