Repository navigation
[mypyc] Intern a few more strings - #20537
ilevkivskyi merged 1 commit into
Conversation
Sorry, something went wrong.
| PyInit_strings(void) | ||
| { | ||
| intern_strings(); | ||
| return PyModuleDef_Init(&librt_strings_module); | ||
| } |
There was a problem hiding this comment.
A few of the changed functions changed here are also called by librt. Therefore we need to initialize them here as well, not just for mypyc.
Technically this is a small performance penalty for other projects which only rely on librt. If that's relevant, we could consider separating the strings used by just mypyc and those used by both mypyc and librt.
Overall this will likely still be a net improvement though, since the strings are initialized / converted to a PyObject only once instead of for each function call.
/CC @ilevkivskyi
Sorry, something went wrong.
There was a problem hiding this comment.
What is the expected order of magnitude of the penalty? If it is under 1ms I think it is OK.
Also cc @JukkaL for visibility.
Sorry, something went wrong.
There was a problem hiding this comment.
(to be clear my question is specifically about import time, not the net performance)
Sorry, something went wrong.
There was a problem hiding this comment.
Not sure tbh, though all the function does is create a PyObject for each string in static_data.c and immortalize it. https://github.com/python/mypy/blob/master/mypyc/lib-rt/static_data.c
https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_InternFromString
Sorry, something went wrong.
ilevkivskyi
left a comment
There was a problem hiding this comment.
I think this is OK (and I didn't see any objections).
Sorry, something went wrong.
Followup to #20460