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-140544: Always assume that thread locals are available by ZeroIntensity · Pull Request #140690 · python/cpython

Conversation

@ZeroIntensity

@ZeroIntensity ZeroIntensity commented Oct 27, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Python has required thread local support since 3.12. By assuming that thread locals are always supported, we can improve the performance of third-party extensions by allowing them to access the attached thread and interpreter states directly.

cc @cdce8p

  • Issue: Store current interpreter as a thread local variable #140544

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ZeroIntensity
Always assume the HAVE_THREAD_LOCAL macro is defined.
977e50a
@ZeroIntensity
ZeroIntensity requested review from brettcannon, ericsnowcurrently, kumaraditya303, ncoghlan and warsaw as code owners October 27, 2025 22:36
@ZeroIntensity ZeroIntensity added the skip news label Oct 27, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Oct 27, 2025
Store current interpreter as a thread local variable #140544
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Oct 27, 2025
@ZeroIntensity ZeroIntensity changed the title gh-140544: Always assume the HAVE_THREAD_LOCAL macro is defined. gh-140544: Always assume that thread locals are available Oct 27, 2025
@cdce8p

cdce8p commented Oct 27, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Hmm, I'm still getting the same error I mentioned earlier.

  In file included from /.../cpython/Include/internal/pycore_object.h:12: (diff)
  In file included from /.../cpython/Include/internal/pycore_gc.h:12: (diff)
  /.../cpython/Include/internal/pycore_pystate.h:93:8: error: unknown type name '_Py_thread_local' (diff)
  extern _Py_thread_local PyThreadState *_Py_tss_tstate; (diff)
         ^ (diff)
  /.../cpython/Include/internal/pycore_pystate.h:93:38: error: expected ';' after top level declarator (diff)
  extern _Py_thread_local PyThreadState *_Py_tss_tstate; (diff)
                                       ^ (diff)
                                       ; (diff)
  /.../cpython/Include/internal/pycore_pystate.h:94:8: error: unknown type name '_Py_thread_local' (diff)
  extern _Py_thread_local PyInterpreterState *_Py_tss_interp; (diff)
         ^ (diff)
  /.../cpython/Include/internal/pycore_pystate.h:94:43: error: expected ';' after top level declarator (diff)
  extern _Py_thread_local PyInterpreterState *_Py_tss_interp; (diff)
                                            ^ (diff)
                                            ; (diff)
  /.../cpython/Include/internal/pycore_pystate.h:119:12: error: use of undeclared identifier '_Py_tss_tstate' (diff)
      return _Py_tss_tstate; (diff)
             ^ (diff)
  /.../cpython/Include/internal/pycore_pystate.h:213:12: error: use of undeclared identifier '_Py_tss_interp' (diff)
      return _Py_tss_interp; (diff)
             ^ (diff)
  6 errors generated. (diff)

Somehow the # ifdef Py_BUILD_CORE check fails, even though mypyc defines it.
https://github.com/python/mypy/blob/v1.18.2/mypyc/lib-rt/pythonsupport.h#L16-L18
All reactions

Sorry, something went wrong.

Uh oh!

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

cdce8p commented Oct 27, 2025

Copy link
Copy Markdown
Contributor

Do I need to pass Py_BUILD_CORE through to my local cpython build? This is my current build setup

make clean
./configure --with-openssl=$(brew --prefix openssl@3)
make -j4 -s
All reactions

Sorry, something went wrong.

Uh oh!

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

ZeroIntensity commented Oct 27, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

Hm, you're defining Py_BUILD_CORE after Python.h has been included, so pyport.h isn't seeing it. I'm not sure if that's something we intended to support.
All reactions

Sorry, something went wrong.

Uh oh!

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

cdce8p commented Oct 27, 2025

Copy link
Copy Markdown
Contributor

Hm, you're defining Py_BUILD_CORE after Python.h has been included, so pyport.h isn't seeing it. I'm not sure if that's something we intended to support.

That hasn't been an issue so far. The check here works fine / if I remove Py_BUILD_CORE from mypyc I get the error as expected.

cpython/Include/internal/pycore_mimalloc.h

Lines 4 to 6 in f5394c2

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity
Move _Py_thread_local to pycore_pystate.h
81b7217 
This fixes extensions that define Py_BUILD_CORE after including
Python.h, but before including pycore_pystate.h, such as mypyc.
@ZeroIntensity

ZeroIntensity commented Oct 27, 2025

Copy link
Copy Markdown
Member Author

I just pushed a new commit, where _Py_thread_local is moved to pycore_pystate.h. Can you test that?
cdce8p reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

cdce8p commented Oct 27, 2025

Copy link
Copy Markdown
Contributor

I just pushed a new commit, where _Py_thread_local is moved to pycore_pystate.h. Can you test that?

This works now.

--

Hm, you're defining Py_BUILD_CORE after Python.h has been included, so pyport.h isn't seeing it. I'm not sure if that's something we intended to support.

I missed that mypyc is defining Py_BUILD_CORE before including any internal/** headers. Though that's much simpler than doing the same for Python.h which is included nearly everywhere.
ZeroIntensity reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 reviewed Oct 28, 2025
View reviewed changes
Comment thread Include/pyport.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity
Remove Py_BUILD_CORE check in pyport.h
823f0e8
kumaraditya303
kumaraditya303 reviewed Oct 28, 2025
View reviewed changes
Comment thread Include/internal/pycore_pystate.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity added 3 commits October 28, 2025 08:33
@ZeroIntensity
Remove _Py_thread_local macro definition
b2dde90
@ZeroIntensity
Move back to pyport.h
bb61072
@ZeroIntensity
Move definition of _Py_thread_local back to pyport.h
2c70a96
kumaraditya303
kumaraditya303 reviewed Oct 28, 2025
View reviewed changes
Comment thread Include/internal/pycore_pystate.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity
Fix stray newline change
5c85aef
@kumaraditya303

kumaraditya303 commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

@cdce8p Would you please check that mypyc still works after the new changes?
ZeroIntensity and cdce8p reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

cdce8p commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

@cdce8p Would you please check that mypyc still works after the new changes?

Just ran the whole test suite again. Looks good.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 approved these changes Oct 28, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 28, 2025
@ZeroIntensity
ZeroIntensity merged commit 2cefa70 into python:main Oct 28, 2025
4 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity
ZeroIntensity deleted the remove-have-thread-local branch October 28, 2025 13:07
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 28, 2025
@kumaraditya303 kumaraditya303 mentioned this pull request Oct 30, 2025
gh-131253: free-threaded build support for pystats #137189
Merged
@ngoldbaum ngoldbaum mentioned this pull request Dec 1, 2025
python.h setting HAVE_THREAD_LOCAL breaks numpy build #142163
Closed
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@ZeroIntensity @StanFromIreland
pythongh-140544: Always assume that thread locals are available (pyth…
32d3fc5 
…onGH-140690)

Python has required thread local support since 3.12 (see pythonGH-103324). By assuming that thread locals are always supported, we can improve the performance of third-party extensions by allowing them to access the attached thread and interpreter states directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

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

@ZeroIntensity @cdce8p @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.