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-108223: Add --disable-gil to configure by colesbury · Pull Request #108227 · python/cpython

Conversation

@colesbury

@colesbury colesbury commented Aug 21, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

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.

  • Issue: Add --disable-gil option to configure #108223

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
@colesbury
pythongh-108223: Add --disable-gil to configure
057f1eb
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.
@colesbury
colesbury requested review from corona10 and erlend-aasland as code owners August 21, 2023 17:10
@bedevere-bot bedevere-bot mentioned this pull request Aug 21, 2023
Add --disable-gil option to configure #108223
Closed
@bedevere-bot bedevere-bot added the awaiting review label Aug 21, 2023
@colesbury colesbury added skip news awaiting review and removed awaiting review labels Aug 21, 2023
corona10
corona10 approved these changes Aug 21, 2023
View reviewed changes

@corona10 corona10 left a comment

Copy link
Copy Markdown
Member

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.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Aug 21, 2023
@corona10

corona10 commented Aug 21, 2023

Copy link
Copy Markdown
Member

@colesbury Are you ready to be merged or need more additional work?

All reactions

Sorry, something went wrong.

Uh oh!

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

@colesbury

colesbury commented Aug 21, 2023

Copy link
Copy Markdown
Contributor Author

@corona10 thanks! it is ready to merge

corona10 reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

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

@corona10
corona10 enabled auto-merge (squash) August 21, 2023 17:25
@corona10
corona10 merged commit b16ecb8 into python:main Aug 21, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Aug 21, 2023
@vstinner

vstinner commented Aug 21, 2023

Copy link
Copy Markdown
Member

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 :-)

All reactions

Sorry, something went wrong.

Uh oh!

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

@colesbury

colesbury commented Aug 21, 2023

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Aug 21, 2023

Copy link
Copy Markdown
Member

It's just a remark, I'm fine with Py_NOGIL.

colesbury 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.

@colesbury
colesbury deleted the pep703-configure branch August 21, 2023 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10 corona10 approved these changes
@erlend-aasland 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

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.

4 participants

@colesbury @corona10 @vstinner @bedevere-bot
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.