gh-140795: fetch thread state once on fast path for critical sections by kumaraditya303 · Pull Request #141406 · python/cpython · GitHub
Skip to content

gh-140795: fetch thread state once on fast path for critical sections - #141406

Merged
kumaraditya303 merged 3 commits into
python:mainfrom
kumaraditya303:critical-section
Nov 21, 2025
Merged

gh-140795: fetch thread state once on fast path for critical sections#141406
kumaraditya303 merged 3 commits into
python:mainfrom
kumaraditya303:critical-section

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Currently on the fastpath for critical section where the objects are not locked, the thread state is fetched twice once while acquiring it and once while releasing it. This PR optimizes it to fetch it once and store it (in a temp variable on stack) so that on the fastpath thread state is fetched once.
This should help performance in shared modules such as ssl where thread state access is slower because of extra function call to _PyThreadState_GetCurrent.

Comment thread Include/internal/pycore_critical_section.h Outdated
@colesbury

Copy link
Copy Markdown
Contributor

@kumaraditya303

Copy link
Copy Markdown
Contributor Author

How much of a difference does this make to asyncio_tcp_ssl?

It is ~5% faster on macOS with this change.

@kumaraditya303
kumaraditya303 merged commit 49ff8b6 into python:main Nov 21, 2025
50 checks passed
@kumaraditya303
kumaraditya303 deleted the critical-section branch November 21, 2025 14:19
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
ashm-dev pushed a commit to ashm-dev/cpython that referenced this pull request Dec 8, 2025
encukou added a commit to encukou/cpython that referenced this pull request Mar 17, 2026
pythonGH-141406 improved performance by only fetching thread state once
and storing it in a variable on the stack.

This instead puts the thread state in the PyCriticalState struct
(also a temp variable on the stack), bringing the public and private
implementations closer together.
@encukou

encukou commented Mar 17, 2026

Copy link
Copy Markdown
Member

I don't understand why this is only done for Py_BUILD_CORE code. Is there a reason to exclude non-stdlib users of critical sections?

Putting the thread state in a temp stack variable for all users seems to work well: https://github.com/python/cpython/pull/146066/files

@colesbury

Copy link
Copy Markdown
Contributor

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