GeezerVison is showing github.com — gh-108223: Add --disable-gil to configure by colesbury · Pull Request #108227 · python/cpython
Conversation
colesbury
commented
Aug 21, 2023
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
The --disable-gil flags does not do anything yet other than define the Py_NOGIL macro. This is intended to support setting up additional buildbots by @itamaro.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
itamaro, aisk, and mara004 reacted with thumbs up emoji
aisk reacted with hooray emoji
All reactions
-
👍
3 reactions
-
🎉
1 reaction
The `--disable-gil` flags does not do anything yet other than define the
Py_NOGIL macro. This is intended to support setting up additional
buildbots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I both check the following cases, and it works as expected!
- ./configure --disable-gil
- ./configure
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@colesbury Are you ready to be merged or need more additional work?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
corona10 reacted with rocket emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
define the Py_NOGIL macro
I dislike double negation, would it be possible to have a companion macro, like Py_HAVE_GIL? For example, Py_HAVE_GIL can be always defined, as 0 or 1. Or Py_HAVE_GIL would not be defined if Py_NOGIL is defined.
In the long term, I would prefer that Python has no GIL by default, but so we would only have special code for the legacy build where Python has a GIL. Well, maybe that's a politic question :-)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@vstinner, I generally agree with you about double negation, but I think in this case it's more convenient to define a Py_NOGIL macro or equivalent.
- All or nearly all of the special code will be fore running without the GIL. I don't expect any special code for running with the GIL. (Especially because
--disable-gil builds still need to support running with the GIL as a fallback.)
- Previous versions of CPython (and PyPy) wouldn't have the
Py_HAVE_GIL macro defined, so C-API extensions can't rely on it always being defined, even if we do so in 3.13.
- It's a bit more convenient to write
#ifdef Py_NOGIL than #if Py_HAVE_GIL == 0
- The
Py_NOGIL macro matches the PEP draft
None of these reasons are tremendously important, but the choice of macro name doesn't seem tremendously important either.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
It's just a remark, I'm fine with Py_NOGIL.
|
colesbury reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
corona10
corona10 approved these changes
erlend-aasland
Awaiting requested review from erlend-aasland
erlend-aasland is a code owner
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
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.
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.
|