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

gh-84436: Improve Immortalization for Builtin Types #103823

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Apr 25, 2023

We do two things here: immortalize the builtin structseq types, and immortalize tp_dict, tp_bases, and tp_mro for builtin types. This is necessary for a per-interpreter GIL.

Note that we could also take the approach to immortalization that no immortal object should ever be freed, even if allocated dynamically. That's something we'll sort out as soon as we can. In the meantime, the change here takes care of isolation concerns.

@ericsnowcurrently ericsnowcurrently changed the title gh-84436: Immortalized Objects gh-84436: Improve Immortalization for Builtin Types Apr 25, 2023
@ericsnowcurrently ericsnowcurrently added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 25, 2023
ericsnowcurrently added a commit that referenced this pull request May 3, 2023
…3912)

his involves moving tp_dict, tp_bases, and tp_mro to PyInterpreterState, in the same way we did for tp_subclasses.  Those three fields are effectively const for builtin static types (unlike tp_subclasses).  In theory we only need to make their values immortal, along with their contents.  However, that isn't such a simple proposition.  (See gh-103823.)  In the meantime the simplest solution is to move the fields into the interpreter.

One alternative is to statically allocate the values, but that's its own can of worms.
@zware zware removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants