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: Refer to PEP 703 as Free Threading by vstinner · Pull Request #112780 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Dec 5, 2023 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Rename support.Py_GIL_DISABLED to support.FREE_THREADING.

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

📚 Documentation preview 📚: https://cpython-previews--112780.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
vstinner requested review from brettcannon, corona10, ericsnowcurrently, erlend-aasland, gvanrossum, markshannon, ncoghlan and warsaw as code owners December 5, 2023 23:18
@bedevere-app bedevere-app Bot added the awaiting core review label Dec 5, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Dec 5, 2023
Add --disable-gil option to configure #108223
Closed
@vstinner

vstinner commented Dec 5, 2023

Copy link
Copy Markdown
Member Author

cc @colesbury @corona10

Should the Py_GIL_DISABLED macro be renamed to Py_FREE_THREADING?

See the steering council decision: python/steering-council#221 (comment)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner added the skip news label Dec 5, 2023
@brettcannon
brettcannon removed their request for review December 5, 2023 23:23
@corona10

corona10 commented Dec 5, 2023

Copy link
Copy Markdown
Member

cc @hugovk
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
corona10
corona10 reviewed Dec 5, 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.

We already changed the term from Py_NOGIL to Py_GIL_DISABLED.
Do we have to touch this term all things one more time?

See PRs of #111863

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@corona10

corona10 commented Dec 5, 2023

Copy link
Copy Markdown
Member

Ah okay, python/steering-council#221 (comment) was announced 2 hours ago.
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Dec 5, 2023

Copy link
Copy Markdown
Member Author

Do we have to touch this term all things one more time?

Naming is hard :-) The update is that the Steering Council took a decision. But this PR is not about Py_GIL_DISABLED macro which can be renamed in a following PR if needed.
All reactions

Sorry, something went wrong.

Uh oh!

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

itamaro commented Dec 6, 2023

Copy link
Copy Markdown
Contributor

I'm confused. Didn't the SC decision explicitly mention that Py_GIL_DISABLED is ok?
hugovk 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.
@corona10

corona10 commented Dec 6, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

I have the same opinion as @itamaro
Except for updating build information from nogil to freethreading, the rest of the things do not need to be updated.
All reactions

Sorry, something went wrong.

Uh oh!

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

hugovk commented Dec 6, 2023

Copy link
Copy Markdown
Member

Should the Py_GIL_DISABLED macro be renamed to Py_FREE_THREADING?

See the steering council decision: python/steering-council#221 (comment)

No, we keep Py_GIL_DISABLED for the macro.

The SC said yesterday:

Within code, we can use technical implementation detail terms in names. Thus #214's Py_GIL_DISABLED #define and the --disable-gil configure flag making sense. Those are not typically exposed to users.

python/steering-council#221 (comment)

This PR doesn't touch the macro though. It changes a constant used by unit tests. This isn't exposed to users, so we could keep it as is, but are also free to change it.

However, because the constant is directly looking up the macro value from sysconfig, I'd think I'd prefer to keep them the same.
corona10 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.
hugovk
hugovk reviewed Dec 6, 2023
View reviewed changes
Comment thread Doc/using/configure.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
hugovk
hugovk reviewed Dec 6, 2023
View reviewed changes
Comment thread Lib/test/libregrtest/utils.py Outdated
# --disable-gil
if sysconfig.get_config_var('Py_GIL_DISABLED'):
build.append("nogil")
build.append("freethreading")

@hugovk hugovk Dec 6, 2023

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.

?

Suggested change
build.append("freethreading")
build.append("gil_disabled")

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
pythongh-108223: Refer to PEP 703 as Free Threading
87709aa
@vstinner
vstinner force-pushed the nonogil branch from c85e092 to 87709aa Compare December 6, 2023 08:45
@vstinner

vstinner commented Dec 6, 2023

Copy link
Copy Markdown
Member Author

I updated the PR. Please review it again.

No, we keep Py_GIL_DISABLED for the macro.

Oh ok. I read the announcement too quickly, I missed the part about Py_GIL_DISABLED.

However, because the constant is directly looking up the macro value from sysconfig, I'd think I'd prefer to keep them the same.

Ok, I updated my PR for that.

@hugovk proposed build.append("gil_disabled"). From what I understood, anything displayed to the user should not associate free threading with negative term such as "disabled". I prefer to reuse the "official term": "free threading". I added an underscore for readability: build.append("free_threading").
corona10 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.
corona10
corona10 approved these changes Dec 6, 2023
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Dec 6, 2023
@vstinner
vstinner merged commit f885263 into python:main Dec 6, 2023
@vstinner
vstinner deleted the nonogil branch December 6, 2023 11:55
@bedevere-app bedevere-app Bot removed the awaiting merge label Dec 6, 2023
@vstinner

vstinner commented Dec 6, 2023

Copy link
Copy Markdown
Member Author

Merged, thanks for reviews.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@vstinner @aisk
pythongh-108223: Refer to PEP 703 as Free Threading (python#112780)
1a8ae85
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@vstinner @Glyphack
pythongh-108223: Refer to PEP 703 as Free Threading (python#112780)
66501b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk left review comments
@corona10 corona10 corona10 approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently
@ncoghlan ncoghlan Awaiting requested review from ncoghlan
@warsaw warsaw Awaiting requested review from warsaw
@markshannon markshannon Awaiting requested review from markshannon
@gvanrossum gvanrossum Awaiting requested review from gvanrossum
@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

@vstinner @corona10 @itamaro @hugovk
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.