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-116167: Allow disabling the GIL with PYTHON_GIL=0 or -X gil=0 #116338

Merged
merged 11 commits into from Mar 11, 2024

Conversation

swtaarrs
Copy link
Member

@swtaarrs swtaarrs commented Mar 4, 2024

In free-threaded builds, running with PYTHON_GIL=0 or -X gil=0 will now disable the GIL. #116322 and #116329 track follow-up work to re-enable the GIL when loading an incompatible extension, and to disable the GIL by default, respectively.

In order to support re-enabling the GIL at runtime, all GIL-related data structures are initialized as usual, and disabling the GIL simply sets a flag at startup that causes take_gil() and drop_gil() to return early.

With PYTHON_GIL=0 set, I spot-checked a few tests and small programs that don't use threads. They all seem to run fine, and very basic threaded programs work, sometimes. Trying to run the full test suite crashes pretty quickly, in test_asyncio.

@swtaarrs
Copy link
Member Author

swtaarrs commented Mar 4, 2024

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @swtaarrs for commit ac24233 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • AMD64 Ubuntu NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR

In free-threaded builds, running with `PYTHON_GIL=0` will now disable the
GIL. Follow-up issues track work to re-enable the GIL when loading an
incompatible extension, and to disable the GIL by default.

In order to support re-enabling the GIL at runtime, all GIL-related data
structures are initialized as usual, and disabling the GIL simply sets a flag
that causes `take_gil()` and `drop_gil()` to return early.

With `PYTHON_GIL=0` set, I spot-checked a few tests and small programs that
don't use threads. They all seem to run fine, and very basic threaded programs
work, sometimes. Trying to run the full test suite crashes pretty quickly, in
`test_asyncio`.
@swtaarrs
Copy link
Member Author

swtaarrs commented Mar 5, 2024

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @swtaarrs for commit 9e03999 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • ARM64 MacOS M1 NoGIL PR

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to add test case in this file too.

def test_int_max_str_digits(self):

@corona10
Copy link
Member

corona10 commented Mar 5, 2024

Ah ignore above comments, this PR only referencing the environment variable so not proper for this case.

You can just add the testcase for the environment variable, still worth to adding it.

@corona10
Copy link
Member

corona10 commented Mar 5, 2024

At not for this PR, since this feature is not officially announced, but in some moment, you should write about the environment variable at https://github.com/python/cpython/blob/72714c0266ce6d39c7c7fb63f617573b8f5a3cb2/Doc/using/cmdline.rst to get user feedback.

@swtaarrs
Copy link
Member Author

swtaarrs commented Mar 5, 2024

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @swtaarrs for commit 4f43a87 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • AMD64 Windows Server 2022 NoGIL PR

@swtaarrs swtaarrs marked this pull request as ready for review March 5, 2024 22:56
@swtaarrs swtaarrs requested a review from vstinner March 5, 2024 23:16
@swtaarrs
Copy link
Member Author

swtaarrs commented Mar 5, 2024

you should write about the environment variable ... to get user feedback

What would be a good time to do that? Maybe once the GIL is disabled by default, so the description doesn't have to change?

Python/initconfig.c Outdated Show resolved Hide resolved
@colesbury
Copy link
Contributor

colesbury commented Mar 6, 2024

I would document the environment variable now, when it's added. For reference, we already document the --disable-gil configure flag.

I don't think it needs to be extensive, but should cover:

  • Only available in free-threaded builds
  • 0 forces the GIL to be disabled
  • 1 forces the GIL to be enabled
  • New in version 3.13

The behavior when then environment variable is not set is what's going to change in the future. We can document that later.

@swtaarrs
Copy link
Member Author

swtaarrs commented Mar 7, 2024

After discussion in Discord, I'm also going to add a -X option to go with the environment variable.

@swtaarrs swtaarrs marked this pull request as draft March 7, 2024 14:31
@swtaarrs swtaarrs changed the title gh-116167: Allow disabling the GIL with PYTHON_GIL=0 gh-116167: Allow disabling the GIL with PYTHON_GIL=0 or -X gil=0 Mar 7, 2024
@swtaarrs swtaarrs marked this pull request as ready for review March 7, 2024 22:29
@swtaarrs swtaarrs requested a review from gpshead as a code owner March 7, 2024 22:29
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I've left one minor comment about an unclear test. It really isn't a blocker, as the test is passing, but it would still be nice to have a response.

Lib/test/_test_embed_set_config.py Outdated Show resolved Hide resolved
@colesbury colesbury merged commit 2731913 into python:main Mar 11, 2024
34 checks passed
@vstinner
Copy link
Member

This is both exciting and very scary. Congrats @swtaarrs and @colesbury :-)

@alikud

This comment was marked as off-topic.

@edpyt

This comment was marked as off-topic.

@fractalvision

This comment was marked as off-topic.

@python python locked as spam and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants