bpo-40077: Convert _abc module to use PyType_FromSpec() by corona10 · Pull Request #19202 · python/cpython · GitHub

Replace statically allocated types with heap allocated types:
use PyType_FromSpec().

Add a module state to store the _abc_data_type..
Add traverse, clear and free functions to the module.

https://bugs.python.org/issue40077

Replace statically allocated types with heap allocated types: use PyType_FromSpec(). Add a module state to store the _abc_data_type.. Add traverse, clear and free functions to the module.

I am not sure that the abc_invalidation_counter is okay to be shared as static from interpreter Isolation point of view.

Should we move this into PyInterpreterState?

It seems like it would be safe to put it in the module state. The purpose of the counter is to invalidate the module cache. Registering a class in module instance 1 should not invalidate module instance 2. What do you think?

I agree, I 've updated the PR.

This test might be what we wanted exactly.
I just updated the test for more validation.

It seems like you cannot access to the module state in abc_data_new() which prevents you to move abc_invalidation_counter into the module state :-(

I suggest to leave abc_invalidation_counter as a "static" variable, but please add a FIXME like:

/* FIXME: PEP 573: Move abc_invalidation_counter into _abcmodule_state */

It's safe to keep it as a static variable until each interpreter gets its own GIL.

Until it happens, the change should move the module closer to the goal of isolated interpreters.

@vstinner
Thanks for the review.

PEP 573: Move abc_invalidation_counter into _abcmodule_state.

I agree, PEP 573 will solve this issue.
I 've updated the PR. Please take a look.

Co-Authored-By: Victor Stinner <vstinner@python.org>

@vstinner Updated! Please take a look

LGTM. Sorry, just one last change request, I promise ;-)

Thanks, I merged your PR.

bpo-40077: Convert _abc module to use PyType_FromSpec()

47eb700

Replace statically allocated types with heap allocated types: use PyType_FromSpec(). Add a module state to store the _abc_data_type.. Add traverse, clear and free functions to the module.

the-knights-who-say-ni added the CLA signed label Mar 28, 2020

bedevere-bot added the awaiting review label Mar 28, 2020

corona10 commented Mar 28, 2020

View reviewed changes

corona10 requested a review from vstinner March 28, 2020 05:47

bpo-40077: rename

7756065

corona10 commented Mar 29, 2020

View reviewed changes

vstinner reviewed Mar 29, 2020

View reviewed changes

Comment thread Modules/_abc.c Outdated

corona10 added 2 commits March 30, 2020 00:51

bpo-40077: Update test_abc.py

ec8ccc6

bpo-40077: Add comment for abc_invalidation_counter.

c2ade98

corona10 force-pushed the bpo-40077-abc branch from 29dabe8 to c2ade98 Compare March 29, 2020 15:53

corona10 requested a review from vstinner March 29, 2020 16:08

corona10 commented Mar 29, 2020

View reviewed changes

vstinner reviewed Mar 29, 2020

View reviewed changes

Comment thread Modules/_abc.c Outdated

Comment thread Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst Outdated

corona10 and others added 2 commits March 30, 2020 11:29

Update Modules/_abc.c

0c0a895

Co-Authored-By: Victor Stinner <vstinner@python.org>

bpo-40077: Remove NEWS.d

32eaf52

corona10 added the skip news label Mar 30, 2020

vstinner reviewed Mar 30, 2020

View reviewed changes

Comment thread Modules/_abc.c

bpo-40077: Apply code review.

98b9300

vstinner reviewed Mar 30, 2020

View reviewed changes

Comment thread Modules/_abc.c Outdated

bpo-40077: Update the type of _abc_data_type.

14c3e52

vstinner merged commit 53e4c91 into python:master Mar 30, 2020

bedevere-bot removed the awaiting review label Mar 30, 2020

corona10 mentioned this pull request Jun 27, 2022

Convert static types to heap types: use PyType_FromSpec() #84258

Closed

                  negative cache to be cleared before its next use.
                  Note: this counter is private. Use `abc.get_cache_token()` for
                  external code. */
               static unsigned long long abc_invalidation_counter = 0;

bpo-40077: Convert _abc module to use PyType_FromSpec() by corona10 · Pull Request #19202 · python/cpython · GitHub

Uh oh!

Uh oh!

bpo-40077: Convert _abc module to use PyType_FromSpec() - #19202

bpo-40077: Convert _abc module to use PyType_FromSpec()#19202vstinner merged 8 commits intopython:masterpython/cpython:masterfrom corona10:bpo-40077-abccorona10/cpython:bpo-40077-abcCopy head branch name to clipboard

Conversation

corona10 commented Mar 28, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corona10 Mar 28, 2020

Choose a reason for hiding this comment

Uh oh!

corona10 Mar 28, 2020

Choose a reason for hiding this comment

Uh oh!

vstinner Mar 28, 2020

Choose a reason for hiding this comment

Uh oh!

corona10 Mar 29, 2020

Choose a reason for hiding this comment

Uh oh!

corona10 Mar 29, 2020

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

corona10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

corona10 commented Mar 30, 2020

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner commented Mar 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bpo-40077: Convert _abc module to use PyType_FromSpec()#19202
vstinner merged 8 commits into
python:masterfrom
corona10:bpo-40077-abc

corona10 commented Mar 28, 2020 •
edited by bedevere-bot
Loading