GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [3.12] gh-113993: Make interned strings mortal (GH-120520, GH-121364, GH-121903, GH-122303) by encukou · Pull Request #123065 · python/cpython

Conversation

@encukou

@encukou encukou commented Aug 16, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This backports several PRs for gh-113993, making interned strings mortal so they can be garbage-collected when no longer needed.

  • Allow interned strings to be mortal, and fix related issues (gh-113993: Allow interned strings to be mortal, and fix related issues #120520)

    • Add an InternalDocs file describing how interning should work and how to use it.

    • Add internal functions to explicitly request what kind of interning is done:

      • _PyUnicode_InternMortal
      • _PyUnicode_InternImmortal
      • _PyUnicode_InternStatic
    • Switch uses of PyUnicode_InternInPlace to those.

    • Disallow using _Py_SetImmortal on strings directly.
      You should use _PyUnicode_InternImmortal instead:

      • Strings should be interned before immortalization, otherwise you're possibly
        interning a immortalizing copy.
      • _Py_SetImmortal doesn't handle the SSTATE_INTERNED_MORTAL to
        SSTATE_INTERNED_IMMORTAL update, and those flags can't be changed in
        backports, as they are now part of public API and version-specific ABI.
    • Add private _only_immortal argument for sys.getunicodeinternedsize, used in refleak test machinery.

    Make sure the statically allocated string singletons are unique. This means these sets are now disjoint:

    • _Py_ID
    • _Py_STR (including the empty string)
    • one-character latin-1 singletons

    Now, when you intern a singleton, that exact singleton will be interned.

    • Add a _Py_LATIN1_CHR macro, use it instead of _Py_ID/_Py_STR for one-character latin-1 singletons everywhere (including Clinic).

    • Intern _Py_STR singletons at startup.

    • Beef up the tests. Cover internal details (marked with @cpython_only).

    • Add lots of assertions

  • Don't immortalize in PyUnicode_InternInPlace; keep immortalizing in other API (gh-113993: Don't immortalize in PyUnicode_InternInPlace; keep immortalizing in other API #121364)

    • Switch PyUnicode_InternInPlace to _PyUnicode_InternMortal, clarify docs

    • Document immortality in some functions that take const char *

    This is PyUnicode_InternFromString;
    PyDict_SetItemString, PyObject_SetAttrString;
    PyObject_DelAttrString; PyUnicode_InternFromString;
    and the PyModule_Add convenience functions.

    Always point out a non-immortalizing alternative.

    • Don't immortalize user-provided attr names in _ctypes
  • Immortalize names in code objects to avoid crash (gh-121863: Immortalize names in code objects to avoid crash #121903)

  • Intern latin-1 one-byte strings at startup (gh-122291: Intern latin-1 one-byte strings at startup #122303)

There are some 3.12-specific changes, mainly to allow statically allocated strings in deepfreeze. (In 3.13, deepfreeze switched to the general _Py_ID/_Py_STR.)

Co-authored-by: Eric Snow [email protected]


📚 Documentation preview 📚: https://cpython-previews--123065.org.readthedocs.build/

Issue: #113993

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
encukou and others added 7 commits August 16, 2024 13:36
@encukou @ericsnowcurrently
[3.12] pythongh-113993: Allow interned strings to be mortal, and fix …
9fd6334
…related issues (pythonGH-120520)

* Add an InternalDocs file describing how interning should work and how to use it.

* Add internal functions to *explicitly* request what kind of interning is done:
  - `_PyUnicode_InternMortal`
  - `_PyUnicode_InternImmortal`
  - `_PyUnicode_InternStatic`

* Switch uses of `PyUnicode_InternInPlace` to those.

* Disallow using `_Py_SetImmortal` on strings directly.
  You should use `_PyUnicode_InternImmortal` instead:
  - Strings should be interned before immortalization, otherwise you're possibly
    interning a immortalizing copy.
  - `_Py_SetImmortal` doesn't handle the `SSTATE_INTERNED_MORTAL` to
    `SSTATE_INTERNED_IMMORTAL` update, and those flags can't be changed in
    backports, as they are now part of public API and version-specific ABI.

* Add private `_only_immortal` argument for `sys.getunicodeinternedsize`, used in refleak test machinery.

* Make sure the statically allocated string singletons are unique. This means these sets are now disjoint:
  - `_Py_ID`
  - `_Py_STR` (including the empty string)
  - one-character latin-1 singletons

  Now, when you intern a singleton, that exact singleton will be interned.

* Add a `_Py_LATIN1_CHR` macro, use it instead of `_Py_ID`/`_Py_STR` for one-character latin-1 singletons everywhere (including Clinic).

* Intern `_Py_STR` singletons at startup.

* Beef up the tests. Cover internal details (marked with `@cpython_only`).

* Add lots of assertions

Co-authored-by: Eric Snow <[email protected]>
@encukou
Put _Py_SetImmortal assertion in the old place
9d4ade1
@encukou
Create per-interpreter intern dict after init of deep-frozen modules
1b8c12e
@encukou
[3.12] pythongh-113993: Don't immortalize in PyUnicode_InternInPlace;…
aaa8afe
… keep immortalizing in other API (pythonGH-121364)

* Switch PyUnicode_InternInPlace to _PyUnicode_InternMortal, clarify docs

* Document immortality in some functions that take `const char *`

This is PyUnicode_InternFromString;
PyDict_SetItemString, PyObject_SetAttrString;
PyObject_DelAttrString; PyUnicode_InternFromString;
and the PyModule_Add convenience functions.

Always point out a non-immortalizing alternative.

* Don't immortalize user-provided attr names in _ctypes
(cherry picked from commit b4aedb2)
@encukou
[3.12] pythongh-121863: Immortalize names in code objects to avoid cr…
18b9ade
…ash (pythonGH-121903)
@encukou
[3.12] pythongh-122291: Intern latin-1 one-byte strings at startup (p…
1d4a531
…ythonGH-122303)
@encukou
Adjust internal docs for 3.12
cf6a27d
@encukou
encukou requested review from a team, berkerpeksag, erlend-aasland, iritkatriel, isidentical, lysnikolaou, markshannon, methane, pablogsal, rhettinger and tiran as code owners August 16, 2024 12:15
@bedevere-app bedevere-app Bot mentioned this pull request Aug 16, 2024
Interned strings are immortal, despite what the documentation says #113993
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Aug 16, 2024
@rhettinger
rhettinger removed their request for review August 16, 2024 12:44
@encukou encukou mentioned this pull request Aug 19, 2024
Path.exists() - Memory leak #122849
Closed
encukou added 4 commits August 19, 2024 16:44
@encukou
Regenerate the ABI dump (static strings are reorganized)
fcee663
@encukou
Don't link to the missing glossary term immortal.
0b10ee5
@encukou
Merge branch '3.12' into mortal-interns-3.12
c405c0a
@encukou
Regenerate test_frozenmain.h
2640dc8
The string "<module>" is now interned.
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 21, 2024
@bedevere-bot

bedevere-bot commented Aug 21, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit 2640dc8 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 21, 2024
@encukou

encukou commented Aug 22, 2024

Copy link
Copy Markdown
Member Author

The buildbot failures look unrelated.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@methane methane mentioned this pull request Sep 10, 2024
Dict memory leak in Python 3.12.x / msgpack 1.0.x msgpack/msgpack-python#612
Closed
@encukou

encukou commented Sep 23, 2024

Copy link
Copy Markdown
Member Author

@Yhg1s, please review this backport. Sorry about the size!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed Sep 24, 2024
View reviewed changes
Comment thread InternalDocs/string_interning.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Tools/build/generate_global_objects.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Tools/build/generate_global_objects.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra and others added 3 commits September 27, 2024 10:21
@JelleZijlstra @encukou
generate_global_objects.py: Fix name of macro in error message (pytho…
50e1492
…n#124464)

_PyID does not exist but _Py_ID does.
@encukou
Merge in the main branch
8583146
@Yhg1s
Merge branch '3.12' into mortal-interns-3.12
2d91ff9
@encukou encukou mentioned this pull request Sep 27, 2024
Fix typo in InternalDocs/string_interning.md #124699
Merged
@Yhg1s
Yhg1s merged commit 49f6beb into python:3.12 Sep 27, 2024
@bedevere-app bedevere-app Bot removed the awaiting core review label Sep 27, 2024
@encukou
encukou deleted the mortal-interns-3.12 branch September 27, 2024 23:31
@mgorny

mgorny commented Oct 2, 2024

Copy link
Copy Markdown
Contributor

FYI, this is at least missing a backport of 281ffb6, and therefore causing Rust packages to crash on assertions. I've filed #124887.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag berkerpeksag is a code owner
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@tiran tiran Awaiting requested review from tiran
@isidentical isidentical Awaiting requested review from isidentical isidentical is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner
@methane methane Awaiting requested review from methane methane is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@encukou @bedevere-bot @mgorny @JelleZijlstra @Yhg1s
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.