Skip to content

/ cpython

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

Jump to bottom

bpo-40521: Per-interpreter interned strings #20085

Merged
merged 1 commit into from Dec 26, 2020
Merged

bpo-40521: Per-interpreter interned strings #20085

merged 1 commit into from Dec 26, 2020
+43 −83

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented May 14, 2020 •
edited by bedevere-bot

Make the Unicode dictionary of interned strings per-interpreter to
make it compatible with subinterpreters.

https://bugs.python.org/issue40521
@vstinner
Copy link
Member Author

@vstinner vstinner commented May 14, 2020

This change requires https://bugs.python.org/issue39465 to be fixed, and also to make the type method cache compatible with subinterpreters.
@vstinner vstinner force-pushed the vstinner:unicode_interned_subinterp branch from d86790c to 342c3ed May 14, 2020
@vstinner vstinner mentioned this pull request Jun 7, 2020
Merged
@vstinner
Copy link
Member Author

@vstinner vstinner commented Dec 23, 2020

This change requires https://bugs.python.org/issue39465 to be fixed, and also to make the type method cache compatible with subinterpreters.

With this PR, tests using subinterpreters like test_ast.test_subinterpreter() does crash since the method cache is built by default, whereas it shares interned strings between multiple interpreters. MCACHE must not be defined to test this PR, or the method cache must be fixed for subinterpreters.
@vstinner vstinner force-pushed the vstinner:unicode_interned_subinterp branch from 342c3ed to b11a338 Dec 23, 2020
@vstinner
Copy link
Member Author

@vstinner vstinner commented Dec 23, 2020

I rebased my PR on master. It is still a draft: PR #20058 must be merged first, and then the method cache must be fixed for subinterpreter, before we can consider to merge this PR.
This was referenced Dec 23, 2020
Merged
Merged
@vstinner vstinner force-pushed the vstinner:unicode_interned_subinterp branch 2 times, most recently from 0bbf371 to 2420a96 Dec 26, 2020
@vstinner vstinner marked this pull request as ready for review Dec 26, 2020
@vstinner vstinner requested a review from markshannon as a code owner Dec 26, 2020
@vstinner vstinner changed the title [WIP] bpo-40521: Per-interpreter interned strings bpo-40521: Per-interpreter interned strings Dec 26, 2020
@vstinner vstinner force-pushed the vstinner:unicode_interned_subinterp branch from 2420a96 to 5b46b09 Dec 26, 2020
@vstinner
Copy link
Member Author

@vstinner vstinner commented Dec 26, 2020

And I pushed a second fix for test_repl:

_PyUnicode_ClearInterned() now uses PyDict_Next() to no longer allocate memory, to ensure that the interned dictionary is cleared.

It's more an enhancement than a fix. Previously, the code also failed. It's just that previously, we didn't check if interned is NULL at exit, whereas my PR adds such assertion.
@vstinner vstinner force-pushed the vstinner:unicode_interned_subinterp branch from 5b46b09 to 5aea9de Dec 26, 2020
@vstinner
bpo-40521: Per-interpreter interned strings
Loading status checks…
8cb4246 
Make the Unicode dictionary of interned strings compatible with
subinterpreters.

Remove the INTERN_NAME_STRINGS macro in typeobject.c: names are
always now interned (even if EXPERIMENTAL_ISOLATED_SUBINTERPRETERS
macro is defined).

_PyUnicode_ClearInterned() now uses PyDict_Next() to no longer
allocate memory, to ensure that the interned dictionary is cleared.
@vstinner vstinner force-pushed the vstinner:unicode_interned_subinterp branch from 5aea9de to 8cb4246 Dec 26, 2020
@vstinner vstinner merged commit ea25180 into python:master Dec 26, 2020
11 checks passed
11 checks passed
Docs
Details
Check for source changes
Details
Check if generated files are up to date
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20201226.7 succeeded
Details
Travis CI - Pull Request Build Passed
Details
bedevere/issue-number Issue number 40521 found
Details
bedevere/news News entry found in Misc/NEWS.d
@vstinner vstinner deleted the vstinner:unicode_interned_subinterp branch Dec 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markshannon markshannon

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@vstinner @the-knights-who-say-ni @bedevere-bot
You can’t perform that action at this time.