bpo-28254: Add a C-API for controlling the state of the garbage collector by scoder · Pull Request #25687 · python/cpython · GitHub

https://bugs.python.org/issue28254

…ctor.

Looks good, I will review it in more detail today, but please, add an entry to the what's new document for 3.10 in the meantime.

…ions to make it easy for users to ('atomically') know whether they actually changed something and what state to go back to later. Also, returning an "int" may make it easier to add error handling later, if that becomes needed at some point.

Thanks for the PR @scoder! Make sure to merge it before end of the week 😉

I changed the functions to return the previous state. That's a common idiom in C, and I think a useful one in this case.

Bugs are unlikely. IMO using assert() is good enough for _testcapi tests. You can leave the code as it is ;-)

See for example test_refcount_funcs() which uses assert().

I was thinking that assertions can be compiled out, which is decidedly not intended here.

In general yes, but _testcapimodule.c starts with:

/* Always enable assertions */
#undef NDEBUG

Good to know! I'd keep the test as it is though. It doesn't really hurt, I think.

Are the C functions prototypes defined in the right place? Limited API, header file hierarchy, etc.?

… gc.collect() implementation).

…othing if GC is disabled.

Are the C functions prototypes defined in the right place? Limited API, header file hierarchy, etc.?

You added them to the stable ABI and so you should run "regen-limited-abi".

Are the C functions prototypes defined in the right place? Limited API, header file hierarchy, etc.?

You added them to the stable ABI and so you should run "regen-limited-abi".

That added more than the three. I only committed the ones I added here in order to keep the PR clean.
I'll check where the others came from.

@pablogsal: Do we want to document that it's safe to call this function with an exception raised, and that the exception is saved and then restored?

Hum, uncollectable objects are stored in gc.garbage no? Shoud we document that?

The C code uses "unreachable objects that couldn't be collected" in a comment instead of "uncollectable objects". Maybe it's a more explicit definition.

I don't think this C-API function is the right place to document these details. That's what the gc module is for.

That added more than the three. I only committed the ones I added here in order to keep the PR clean.
I'll check where the others came from.

Right, thanks: https://bugs.python.org/issue43795#msg392212

Co-authored-by: Victor Stinner <vstinner@python.org>

LGTM but please fix the Sphinx markup.

Please fix also the indentation.

LGTM.

Merged, thanks @scoder!

I'm not really excited by adding a C API for each Python function, but this one seems common enough to justify adding an API. Also, @pablogsal liked the idea ;-)

Merged, thanks @scoder!

I'm not really excited by adding a C API for each Python function, but this one seems common enough to justify adding an API. Also, @pablogsal liked the idea ;-)

The GC gods we all serve are pleased with the new API ;)

Thanks Victor and Pablo!

I created PR #25693 which shows how useful are these new C API functions ;-) Ok, maybe, it makes the C code a little bit simpler ;-)

35 lines added, 116 lines saved. Seems a good deal.

FYI, this has broken CI
Example: https://github.com/python/cpython/pull/25719/checks?check_run_id=2466874649
Proposed fix:
#25716

Yes, this adds things to Doc/data/stable_abi.dat but not PC/python3dll.c, which is quite wrong.
Meanwhile, I merged the part of PEP-652 that will prevent these from going out of sync (by generating them both from a single manifest), but the CI on this PR ran before that and the merge happened after.

From now on, CI will be fail on the pull request when issues like this happen, but this one's timing was unfortunate.

Ok, then let's merge #25720 to unblock this

bpo-28254: Add a C-API for controlling the state of the garbage colle…

6229a31

…ctor.

scoder requested a review from vstinner April 28, 2021 13:13

scoder requested a review from pablogsal as a code owner April 28, 2021 13:13

the-knights-who-say-ni added the CLA signed label Apr 28, 2021

bedevere-bot added the awaiting core review label Apr 28, 2021

pablogsal reviewed Apr 28, 2021

View reviewed changes

Comment thread Misc/NEWS.d/next/C API/2021-04-28-12-33-44.bpo-28254.a2561e.rst

bpo-28254: Improve markup and add a What's New entry.

bfbadb9

pablogsal reviewed Apr 28, 2021

View reviewed changes

Comment thread Doc/whatsnew/3.10.rst Outdated

pablogsal and others added 2 commits April 28, 2021 14:44

Apply suggestions from code review

f41d269

pablogsal approved these changes Apr 28, 2021

View reviewed changes

Comment thread Modules/_testcapimodule.c Outdated

Comment thread Modules/_testcapimodule.c Outdated

Comment thread Modules/_testcapimodule.c Outdated

bedevere-bot added awaiting merge and removed awaiting core review labels Apr 28, 2021

pablogsal added 3 commits April 28, 2021 14:47

Update Modules/_testcapimodule.c

7b6e320

Update Modules/_testcapimodule.c

b7e4ee3

Update Modules/_testcapimodule.c

4e11560

bpo-28254: Formatting.

e5f0077

vstinner reviewed Apr 28, 2021

View reviewed changes

Comment thread Doc/whatsnew/3.10.rst

vstinner reviewed Apr 28, 2021

View reviewed changes

scoder added 2 commits April 28, 2021 16:26

bpo-28254: Move new functions to a better place (not cutting into the…

c64aa42

… gc.collect() implementation).

Remove an incorrect part of a comment: PyGC_Collect() actually does n…

cbee918

…othing if GC is disabled.

scoder added 2 commits April 28, 2021 16:43

bpo-28254: Add documentation for the PyGC_*() functions.

0f7e608

bpo-28254: Add new PyGC_*() functions to the stable ABI list.

80c3625

vstinner reviewed Apr 28, 2021

View reviewed changes

bpo-28254: Apply suggestions from doc review.

25fb2aa

vstinner reviewed Apr 28, 2021

View reviewed changes

Comment thread Doc/c-api/gcsupport.rst Outdated

Comment thread Doc/c-api/gcsupport.rst Outdated

Comment thread Doc/c-api/gcsupport.rst Outdated

Apply suggestions from doc review

10ad929

Co-authored-by: Victor Stinner <vstinner@python.org>

vstinner approved these changes Apr 28, 2021

View reviewed changes

Comment thread Doc/c-api/gcsupport.rst Outdated

scoder added 2 commits April 28, 2021 17:48

Add a word on how PyGC_Collect() and gc.collect() differ.

1ffe937

Fix version marker.

d517665

vstinner reviewed Apr 28, 2021

View reviewed changes

Fix version marker (II).

84fb2af

vstinner approved these changes Apr 28, 2021

View reviewed changes

vstinner merged commit 3cc481b into python:master Apr 28, 2021

bedevere-bot removed the awaiting merge label Apr 28, 2021

scoder deleted the bpo-28254_gc_capi branch April 28, 2021 16:12

This was referenced Apr 29, 2021

bpo-43933: Show frame.f_lineno as None, rather than -1, if there is no line number. #25717

Merged

bpo-43976: add vendor config #25718

Closed

bpo-43933: Set frame.f_lineno during call to __exit__ #25719

Merged

+                 If the garbage collector is disabled or already running,
+                 returns ``0`` immediately.
+                 Errors during garbage collection are ignored and printed.
+                 This function does not raise exceptions.

+                  int old_state;
+                  old_state = PyGC_Enable();
+                  msg = "Enable(1)";

+              .. c:function:: Py_ssize_t PyGC_Collect(void)
+                 Performs a garbage collection, if the garbage collector is enabled.
+                 Returns the number of collected + uncollectable objects.

                  Returns the previous state, 0 for disabled and 1 for enabled.
-              .. versionchanged:: 3.10
+              .. versionadded:: 3.10

bpo-28254: Add a C-API for controlling the state of the garbage collector by scoder · Pull Request #25687 · python/cpython · GitHub

Repository navigation

bpo-28254: Add a C-API for controlling the state of the garbage collector - #25687

bpo-28254: Add a C-API for controlling the state of the garbage collector#25687vstinner merged 17 commits intopython:masterpython/cpython:masterfrom scoder:bpo-28254_gc_capiscoder/cpython:bpo-28254_gc_capiCopy head branch name to clipboard

Conversation

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

Uh oh!

Uh oh!

pablogsal commented Apr 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pablogsal commented Apr 28, 2021

Uh oh!

scoder commented Apr 28, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scoder commented Apr 28, 2021

Uh oh!

vstinner commented Apr 28, 2021

Uh oh!

scoder commented Apr 28, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner commented Apr 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Apr 28, 2021

Uh oh!

pablogsal commented Apr 28, 2021

Uh oh!

scoder commented Apr 28, 2021

Uh oh!

vstinner commented Apr 28, 2021

Uh oh!

scoder commented Apr 28, 2021

Uh oh!

markshannon commented Apr 29, 2021

Uh oh!

encukou commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablogsal commented Apr 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

bpo-28254: Add a C-API for controlling the state of the garbage collector#25687
vstinner merged 17 commits into
python:masterfrom
scoder:bpo-28254_gc_capi

scoder commented Apr 28, 2021 •
edited by bedevere-bot
Loading

encukou commented Apr 29, 2021 •
edited
Loading