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-99113: Add a check for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED by ericsnowcurrently · Pull Request #104206 · python/cpython

Conversation

@ericsnowcurrently

@ericsnowcurrently ericsnowcurrently commented May 5, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Py_MOD_PER_INTERPRETER_GIL_SUPPORTED is a new supported value for Py_mod_multiple_interpreters, added in gh-104205.

(This is blocked until PyInterpreterState.ceval.own_gil is added in gh-104204.)

  • Issue: Implement PEP 684: A Per-Interpreter GIL #99113

Sorry, something went wrong.

Uh oh!

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

All reactions
@ericsnowcurrently ericsnowcurrently added the skip news label May 5, 2023
@ericsnowcurrently
ericsnowcurrently requested a review from encukou May 5, 2023 16:59
@ericsnowcurrently
ericsnowcurrently requested review from a team, 1st1, AlexWaygood, Fidget-Spinner, JelleZijlstra, abalkin, asvetlov, berkerpeksag, brettcannon, corona10, erlend-aasland, gpshead, gvanrossum, isidentical, kumaraditya303, ncoghlan, pablogsal, pganssle, rhettinger, tiran, warsaw and willingc as code owners May 5, 2023 16:59
@bedevere-bot bedevere-bot mentioned this pull request May 5, 2023
Implement PEP 684: A Per-Interpreter GIL #99113
Closed
@bedevere-bot bedevere-bot added the awaiting core review label May 5, 2023
@ericsnowcurrently
ericsnowcurrently removed request for a team, berkerpeksag, brettcannon and gpshead May 5, 2023 16:59
@ericsnowcurrently
ericsnowcurrently removed request for AlexWaygood, isidentical and kumaraditya303 May 5, 2023 16:59
@ericsnowcurrently ericsnowcurrently mentioned this pull request May 5, 2023
gh-99113: A Per-Interpreter GIL! #99114
Closed
2 of 3 tasks
@ericsnowcurrently
ericsnowcurrently force-pushed the own-gil-extension-check branch from 9cd8840 to 78b6ae7 Compare May 5, 2023 20:19
@ericsnowcurrently ericsnowcurrently removed the skip news label May 5, 2023
@ericsnowcurrently ericsnowcurrently mentioned this pull request May 5, 2023
gh-99113: Add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED #104205
Merged
erlend-aasland
erlend-aasland reviewed May 5, 2023
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

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

ericsnowcurrently added a commit that referenced this pull request May 5, 2023
@ericsnowcurrently
gh-99113: Add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (gh-104205)
a9c6e06
Here we are doing no more than adding the value for Py_mod_multiple_interpreters and using it for stdlib modules.  We will start checking for it in gh-104206 (once PyInterpreterState.ceval.own_gil is added in gh-104204).
ericsnowcurrently added 5 commits May 5, 2023 15:14
@ericsnowcurrently
Add _PyInterpreterConfig.own_gil.
a73f36f
@ericsnowcurrently
Use PyInterpreterConfig.own_gil.
37bf29c
@ericsnowcurrently
Fail unsupported extensions.
e32c487
@ericsnowcurrently
Add a test.
51d8587
@ericsnowcurrently
Add a NEWS entry.
e2ddc27
@ericsnowcurrently
ericsnowcurrently force-pushed the own-gil-extension-check branch from 78b6ae7 to e2ddc27 Compare May 5, 2023 21:16
@ericsnowcurrently
Merge branch 'main' into own-gil-extension-check
fee34e0
erlend-aasland
erlend-aasland approved these changes May 6, 2023
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

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

BTW, the branch in lines 295-300 seems lack coverage (disclaimer: I only ran test_import)

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Modules/_testmultiphase.c Resolved

Uh oh!

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

Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

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

@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels May 6, 2023
ericsnowcurrently added 3 commits May 6, 2023 14:48
@ericsnowcurrently
Add a note.
ac95d09
@ericsnowcurrently
Add a test for when an extension has multiple Py_mod_create slots.
5ccc220
@ericsnowcurrently
Add a test for when an extension has multiple Py_mod_multiple_interpr…
376217a
…eters slots.
@ericsnowcurrently

ericsnowcurrently commented May 6, 2023

Copy link
Copy Markdown
Member Author

Thanks for pointing out about the missing test coverage. I've added it. I also did likewise for the similar case where Py_mod_create is provided multiple times, which did not have test coverage either.

erlend-aasland reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented May 6, 2023

Copy link
Copy Markdown
Contributor

Thanks for pointing out about the missing test coverage. I've added it. I also did likewise for the similar case where Py_mod_create is provided multiple times, which did not have test coverage either.

Nice, thanks! I can confirm that those branches are now covered.

ericsnowcurrently reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently
ericsnowcurrently enabled auto-merge (squash) May 6, 2023 21:52
@ericsnowcurrently
ericsnowcurrently merged commit fff193b into python:main May 6, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 6, 2023
@ericsnowcurrently
ericsnowcurrently deleted the own-gil-extension-check branch May 6, 2023 21:57
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@ericsnowcurrently @jbower-fb
pythongh-99113: Add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (pythongh-10…
7192a46
…4205)

Here we are doing no more than adding the value for Py_mod_multiple_interpreters and using it for stdlib modules.  We will start checking for it in pythongh-104206 (once PyInterpreterState.ceval.own_gil is added in pythongh-104204).
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@ericsnowcurrently @jbower-fb
pythongh-99113: Add a check for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (p…
fc1e63f
…ythongh-104206)

Py_MOD_PER_INTERPRETER_GIL_SUPPORTED is a new supported value for Py_mod_multiple_interpreters, added in pythongh-104205.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes
@encukou encukou Awaiting requested review from encukou

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

3 participants

@ericsnowcurrently @erlend-aasland @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.