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 — gh-94673: Add Per-Interpreter Storage for Static Builtin Types by ericsnowcurrently · Pull Request #95255 · python/cpython

Conversation

@ericsnowcurrently

@ericsnowcurrently ericsnowcurrently commented Jul 25, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This is the last precursor to storing tp_subclasses (and tp_weaklist) on the interpreter state for static builtin types.

Here we add per-type storage on PyInterpreterState, but only for the static builtin types. This involves the following:

  • add `PyInterpreterState.types'
    • move PyInterpreterState.type_cache to it
    • add a num_builtins_initialized field
    • add a builtins field (a static array big enough for all the static builtin types)
  • add _PyStaticType_GetState() to look up a static builtin type's state
  • add PyTypeObject.tp_static_builtin_index (to hold the type's index into PyInterpreterState.types.builtins)

(Adding an extra field to PyTypeObject isn't ideal and I may change it to store the index somewhere else.)

  • Issue: [subinterpreters] Static types incorrectly share some objects between interpreters #94673

Sorry, something went wrong.

Uh oh!

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

All reactions
ericsnowcurrently and others added 14 commits July 25, 2022 14:29
@ericsnowcurrently
Add PyInterpreterState.types.
7843524
@ericsnowcurrently
Add PyInterpreterState.types.num_builtins_initialized.
09fd0be
@ericsnowcurrently
Hard-code _Py_NUM_STATIC_BUILTIN_TYPES.
afbde1a
@ericsnowcurrently
Store per-interpreter state for static builtin types.
f3aa649
@ericsnowcurrently
Export _PyStaticType_GetState().
9a1cc36
@ericsnowcurrently
Move _PyStaticType_GetState() down next to _PyStaticType_InitBuiltin().
375b2d9
@kumaraditya303 @ericsnowcurrently
make it compile
b0b32a6
@kumaraditya303 @ericsnowcurrently
fix test_sys
eb05d9b
@ericsnowcurrently
Reset tp_static_builtin_index only if a static builtin type.
f5ebd23
@ericsnowcurrently
Factor out get_static_builtin_index() and set_static_builtin_index().
d94818b
@ericsnowcurrently
Reset state->type later on.
06799d5
@ericsnowcurrently
Add some asserts.
d78f10d
@ericsnowcurrently
Decrement num_builtins_initialized.
44edc49
@ericsnowcurrently
Verify the per-interpreter type state has been cleared.
bc7fa26
@ericsnowcurrently ericsnowcurrently added the skip news label Jul 25, 2022
@ericsnowcurrently
ericsnowcurrently requested a review from kumaraditya303 July 25, 2022 20:44
@ericsnowcurrently
ericsnowcurrently requested a review from markshannon as a code owner July 25, 2022 20:44
@bedevere-bot bedevere-bot added the awaiting core review label Jul 25, 2022
@ericsnowcurrently

ericsnowcurrently commented Jul 25, 2022

Copy link
Copy Markdown
Member Author

This was split out of #94995.

All reactions

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently
ericsnowcurrently removed the request for review from markshannon July 25, 2022 20:45
@ericsnowcurrently ericsnowcurrently mentioned this pull request Jul 25, 2022
[subinterpreters] Static types incorrectly share some objects between interpreters #94673
Closed
@ericsnowcurrently
Add tp_static_builtin_index to the docs.
3f20f4a
@ericsnowcurrently

ericsnowcurrently commented Jul 25, 2022

Copy link
Copy Markdown
Member Author

For now consider PyTypeObject.tp_static_builtin_index a temporary addition. I'd like to drop it once we start storing tp_subclasses on PyInterpreterState for static builtin types. At that point we can use tp_subclasses to store the index.

All reactions

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently
Revert "Add tp_static_builtin_index to the docs."
9f7775d
This reverts commit 3f20f4a.
@ericsnowcurrently

ericsnowcurrently commented Jul 25, 2022

Copy link
Copy Markdown
Member Author

I'm going to leave tp_static_builtin_index undocumented since it is temporary. If that changes then we'll document it.

All reactions

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently ericsnowcurrently added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 25, 2022
@bedevere-bot

bedevere-bot commented Jul 25, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @ericsnowcurrently for commit 9f7775d 🤖

If you want to schedule another build, you need to add the ":hammer: 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 Jul 25, 2022
ericsnowcurrently added 2 commits July 26, 2022 11:37
@ericsnowcurrently
Update tp_flags sooner in _PyStaticType_InitBuiltin().
cf04c2c
@ericsnowcurrently
Move the state/index-related helpers to the top.
38427a0
ericsnowcurrently added 9 commits July 26, 2022 12:04
@ericsnowcurrently
Add static_builtin_index_is_set() and static_builtin_index_clear().
d313dbd
@ericsnowcurrently
Hide the 1-based indexing behind the helper functions.
b4bbfcb
@ericsnowcurrently
Move and edit a comment.
1bd26c4
@ericsnowcurrently
Factor out static_builtin_state_init() and static_builtin_state_clear().
652fcc6
@ericsnowcurrently
Un-initialize the type if PyType_Ready() fails.
d558a46
@ericsnowcurrently
Drop an empty line.
39a8fc7
@ericsnowcurrently
Move a comment.
6019eba
@ericsnowcurrently
static_builtin_type_state -> static_builtin_state (and factor out sta…
f2f10f0
…tic_builtin_state_get())
@ericsnowcurrently
Pass the type to static_builtin_state_get().
cea5d34
@ericsnowcurrently
ericsnowcurrently force-pushed the static-builtin-state branch from fe54d39 to cea5d34 Compare July 26, 2022 20:24
ericsnowcurrently added 3 commits July 26, 2022 14:55
@ericsnowcurrently
Handle non-builtin and non-static types in _PyStaticType_GetState().
4348e3f
@ericsnowcurrently
Use static_builtin_index_is_set() in static_builtin_index_get().
1c8b0c3
@ericsnowcurrently
Restrict _PyStaticType_GetState() to static builtin types.
ae2d440
@ericsnowcurrently
ericsnowcurrently merged commit 47e75a0 into python:main Jul 26, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Jul 26, 2022
@ericsnowcurrently
ericsnowcurrently deleted the static-builtin-state branch July 26, 2022 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

3 participants

@ericsnowcurrently @bedevere-bot @kumaraditya303
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.