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.13] gh-126914: Store the Preallocated Thread State's Pointer in a PyInterpreterState Field #127114

Merged

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Nov 21, 2024

This approach eliminates the originally reported race. It also gets rid of the deadlock reported in gh-96071, so we can remove the workaround added then.

This is mostly a cherry-pick of 1c0a104 (AKA gh-126989). The difference is we add PyInterpreterState.threads_preallocated at the end of PyInterpreterState, instead of adding PyInterpreterState.threads.preallocated. That avoids ABI disruption.

…yInterpreterState Field (pythongh-126989)

This approach eliminates the originally reported race. It also gets rid of the deadlock reported in pythongh-96071, so we can remove the workaround added then.
@ericsnowcurrently
Copy link
Member Author

@Yhg1s, this changes only internal ABI, so we should be fine updating the ABI data file, right?

@Yhg1s
Copy link
Member

Yhg1s commented Nov 25, 2024

Yes, the struct is always allocated by the runtime, and the new field is used only by the runtime, never extension modules, so updating the ABI definition is fine.

@Yhg1s
Copy link
Member

Yhg1s commented Dec 2, 2024

Do you want to get this into 3.13.1? (It's scheduled for tomorrow :)

@ericsnowcurrently ericsnowcurrently enabled auto-merge (squash) December 2, 2024 18:39
@ericsnowcurrently ericsnowcurrently merged commit 219b826 into python:3.13 Dec 2, 2024
38 checks passed
@ericsnowcurrently ericsnowcurrently deleted the backport-1c0a104-3.13 branch December 2, 2024 19:01
colesbury added a commit to colesbury/cpython that referenced this pull request Apr 1, 2025
The 3.13 free threaded build immortalizes certain objects to avoid
reference count contention. In pythongh-127114 the condition was
unintentionally changed to happen when the first thread was created
instead of the first non-main thread. The `interp->gc.immortalize` field
is then cleared again during `_PyGC_Init()`.

Change the condition so that we check if we should immortalize objects
using deferred reference counting whenever a non-main thread is created.
Yhg1s pushed a commit that referenced this pull request Apr 7, 2025
gh-131988: Fix a multithreaded scaling regression

The 3.13 free threaded build immortalizes certain objects to avoid
reference count contention. In gh-127114 the condition was
unintentionally changed to happen when the first thread was created
instead of the first non-main thread. The `interp->gc.immortalize` field
is then cleared again during `_PyGC_Init()`.

Change the condition so that we check if we should immortalize objects
using deferred reference counting whenever a non-main thread is created.
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.

2 participants