Skip to content
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

[3.12] gh-106931: Intern Statically Allocated Strings Globally (gh-107272) #110713

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Oct 11, 2023

We tried this before with a dict and for all interned strings. That ran into problems due to interpreter isolation. However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning. Here we circle back to using a global cache, but only for statically allocated strings. We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter. Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.

(cherry-picked from commit b72947a)

(This supersedes gh-107358.)

…ythongh-107272)

We tried this before with a dict and for all interned strings.  That ran into problems due to interpreter isolation.  However, exclusively using a per-interpreter cache caused some inconsistency that can eliminate the benefit of interning.  Here we circle back to using a global cache, but only for statically allocated strings.  We also use a more-basic _Py_hashtable_t for that global cache instead of a dict.

Ideally we would only have the global cache, but the optional isolation of each interpreter's allocator means that a non-static string object must not outlive its interpreter.  Thus we would have to store a copy of each such interned string in the global cache, tied to the main interpreter.

(cherry-picked from commit b72947a)
@ericsnowcurrently ericsnowcurrently enabled auto-merge (squash) November 27, 2023 23:25
@ericsnowcurrently ericsnowcurrently merged commit 4f71f16 into python:3.12 Nov 27, 2023
24 checks passed
@ericsnowcurrently ericsnowcurrently deleted the backport-b72947a-fix-interned-strings branch November 27, 2023 23:57
@brettcannon
Copy link
Member

This broke the 3.12 WASI build: https://buildbot.python.org/all/#/builders/1124/builds/464/steps/11/logs/stdio .

Opened #112503 to track the fix (which is backporting a change made on main).

ericsnowcurrently added a commit that referenced this pull request Nov 29, 2023
Skip tests if no interpreters module.gh-110713 added some tests that use the interpreters module but did not accommodated builds that don't support subinterpreters (incl. an interpreters module).  We fix that here by catching ImportError and skipping tests as appropriate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants