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-108223: Document --disable-gil flag in configure #108236

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Aug 21, 2023

Adds an entry in the configure documentation for the --disable-gil flag.


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

@colesbury colesbury added docs Documentation in the Doc dir skip news build The build process and cross-build labels Aug 21, 2023
.. cmdoption:: --disable-gil

Enables experimental support for running Python without the global
interpreter lock (GIL).
Copy link
Member

Choose a reason for hiding this comment

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

You should add a reference to the related PEP:

See :pep:`703` "Making the Global Interpreter Lock Optional in CPython".

Doc/using/configure.rst Outdated Show resolved Hide resolved
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the added doc!

Comment on lines 190 to 191
Enables experimental support for running Python without the
:term:`global interpreter lock` (GIL).
Copy link
Member

@AA-Turner AA-Turner Aug 21, 2023

Choose a reason for hiding this comment

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

Should we add a warning admonition emphasising that this is experimental, in development, etc -- mainly to preempt a random twitter recommendation that "No GIL has now been implemented! Rebuild your Python today with --disable-gil..."

Perhaps a fear that won't materialise, though!

A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm okay with adding whatever text you think is helpful. I expect to revisit and update this later in the development cycle based on what's actually integrated.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks Sam! Victor's simpler change I think is better for now--we don't really advertise building from source to end-users anyway, so I on reflection I think we don't need a high level of warning here.

A

Doc/using/configure.rst Outdated Show resolved Hide resolved
@itamaro
Copy link
Contributor

itamaro commented Aug 21, 2023

considering how much "nothing" this flag is currently doing, vs how much hype it could lead to -- perhaps it should be left undocumented for now?

otherwise, i'd add something like "experimental and under active development - does NOT yet provide actual GIL-less capabilities - intended for use only by contributors actively working on implementing PEP-703 and buildbots"

@vstinner
Copy link
Member

considering how much "nothing" this flag is currently doing, vs how much hype it could lead to -- perhaps it should be left undocumented for now?

I disagree. I prefer to provide accurate documentation, because of the hype.

@vstinner vstinner enabled auto-merge (squash) August 21, 2023 20:26
@vstinner
Copy link
Member

This is always room for enhancement, but IMO that's a good start for the documentation :-) Maybe a reference documentation should be added later for any question related to NOGIL. For now, the doc points to the complete PEP.

@vstinner vstinner merged commit 4b32d4f into python:main Aug 21, 2023
20 checks passed
@bedevere-bot
Copy link

There's a new commit after the PR has been approved.

@vstinner: please review the changes made to this pull request.

@colesbury colesbury deleted the pep703-configure-docs branch September 1, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review build The build process and cross-build docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants