Skip to content

gh-140358: Bring back elapsed time and unreachable count to gc debug output#140359

Merged
pablogsal merged 14 commits intopython:mainfrom
pgdr:gc-elapsed-time
Oct 20, 2025
Merged

gh-140358: Bring back elapsed time and unreachable count to gc debug output#140359
pablogsal merged 14 commits intopython:mainfrom
pgdr:gc-elapsed-time

Conversation

@pgdr
Copy link
Contributor

@pgdr pgdr commented Oct 20, 2025 •
edited by bedevere-app bot
Loading

Fixes #140358

This code includes pycore_time.h in gc.c and times the run of _PyGC_Collect. If _PyGC_DEBUG_STATS, then we print the total time used:

E.g.

gc: done, 0 unreachable, 0 uncollectable, 0.0741s elapsed

See also discussion at elapsed time debugging in gc.c at discuss.

@bedevere-app
Copy link

bedevere-app bot commented Oct 20, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app bedevere-app bot mentioned this pull request Oct 20, 2025
Closed
sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
@sergey-miryanov
Copy link
Contributor

sergey-miryanov commented Oct 20, 2025

Please add test to test_gc.py. You have to check that a line with your changes presented in the output.

sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
@pgdr
Copy link
Contributor Author

pgdr commented Oct 20, 2025

I'm not sure if it is appropriate to use the performance counter or the monotonic (raw).

I currently chose monotonic raw, whereas the Python 3.13 version has performance counter.

@sergey-miryanov
Copy link
Contributor

sergey-miryanov commented Oct 20, 2025

I'm not sure if it is appropriate to use the performance counter or the monotonic (raw).

I currently chose monotonic raw, whereas the Python 3.13 version has performance counter.

IMHO it is better to use counter like in 3.13 vesion.

@sergey-miryanov sergey-miryanov changed the title gh-140358: Bring back elapsed time and unreachable count to gc debug gh-140358: Bring back elapsed time and unreachable count to gc debug output Oct 20, 2025
@pgdr
Copy link
Contributor Author

pgdr commented Oct 20, 2025

@sergey-miryanov Thanks a lot for all the comments. I believe all issues you pointed to are addressed now. A test has been added, and I included the checking of the other lines in the test as well.

I also moved to using performance counter.

sergey-miryanov
sergey-miryanov reviewed Oct 20, 2025
View reviewed changes
@sergey-miryanov
Copy link
Contributor

sergey-miryanov commented Oct 20, 2025 •
edited
Loading

You need to use the requires_gil_enabled decorator to avoid tests run on the free-threading build.

@pgdr
Copy link
Contributor Author

pgdr commented Oct 20, 2025

@efimov-mikhail I believe I have addressed all your comments, but I still have a doubt regarding including pycore_time. Is it correct as it is now without the include?

@efimov-mikhail
Copy link
Member

efimov-mikhail commented Oct 20, 2025 •
edited
Loading

@efimov-mikhail I believe I have addressed all your comments, but I still have a doubt regarding including pycore_time. Is it correct as it is now without the include?

I think so. CI is green for this PR. And we don't include this file in Python/gc.c on 3.13.
So, I don't see any reason to include this file.

And PyTime_t is defined here:

cpython/Include/cpython/pytime.h

Line 10 in 237dca5

typedef int64_t PyTime_t;

It's part of the public Python C API, as well as PyTime_PerfCounterRaw and PyTime_AsSecondsDouble.

efimov-mikhail
efimov-mikhail approved these changes Oct 20, 2025
View reviewed changes
Copy link
Member

@efimov-mikhail efimov-mikhail 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 PR!

But we have to wait for some Core Developers.

@nascheme
Copy link
Member

nascheme commented Oct 20, 2025

This looks okay to me.

@pablogsal pablogsal merged commit b2f9fb9 into python:main Oct 20, 2025
49 checks passed
@pablogsal
Copy link
Member

pablogsal commented Oct 20, 2025

This should be probably backported. @nascheme WDYT?

@sergey-miryanov
Copy link
Contributor

sergey-miryanov commented Oct 21, 2025

@pablogsal @nascheme Thanks!

@pgdr
Copy link
Contributor Author

pgdr commented Oct 21, 2025

This should be probably backported. @nascheme WDYT?

I would be very happy if this found its way to 3.14 as it would help me debugging my main issue #139951.

@efimov-mikhail
Copy link
Member

efimov-mikhail commented Oct 21, 2025 •
edited
Loading

IMO, we can consider this a bug, since removing elapsed line wasn't intended.
I'll add 3.14 label, because we can simply close the backport PR if we decided to.

@pablogsal @nascheme
I hope that's appropriate.

@efimov-mikhail efimov-mikhail added 3.14 bugs and security fixes needs backport to 3.13 bugs and security fixes labels Oct 21, 2025
@miss-islington-app
Copy link

miss-islington-app bot commented Oct 21, 2025

Thanks @pgdr for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington-app
Copy link

miss-islington-app bot commented Oct 21, 2025

Sorry, @pgdr and @pablogsal, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker b2f9fb9db29296410f76009ac65cf81afa8a92a9 3.13

@efimov-mikhail efimov-mikhail added needs backport to 3.14 bugs and security fixes and removed needs backport to 3.13 bugs and security fixes labels Oct 21, 2025
@miss-islington-app
Copy link

miss-islington-app bot commented Oct 21, 2025

Thanks @pgdr for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@efimov-mikhail efimov-mikhail removed the 3.14 bugs and security fixes label Oct 21, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 21, 2025
@pgdr @miss-islington
pythongh-140358: Bring back elapsed time and unreachable count to gc …
4f6cbee 
…debug output (pythonGH-140359)

(cherry picked from commit b2f9fb9)

Co-authored-by: Pål Grønås Drange <paal.drange+pgdr@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2025

GH-140405 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Oct 21, 2025
@efimov-mikhail
Copy link
Member

efimov-mikhail commented Oct 21, 2025

Sorry, some misclicks.

@nascheme
Copy link
Member

nascheme commented Oct 21, 2025

FWIW, I think backporting is a good idea. It's a small change and it only affects people who turn this option on. Thanks for fixing this.

hugovk pushed a commit that referenced this pull request Oct 27, 2025
@miss-islington @pgdr
[3.14] gh-140358: Bring back elapsed time and unreachable count to gc…
c1bfd4c 
… debug output (GH-140359) (#140405)

Co-authored-by: Pål Grønås Drange <paal.drange+pgdr@gmail.com>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@pgdr @StanFromIreland
pythongh-140358: Bring back elapsed time and unreachable count to gc …
b75aadc 
…debug output (python#140359)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergey-miryanov sergey-miryanov sergey-miryanov approved these changes

@efimov-mikhail efimov-mikhail efimov-mikhail approved these changes

@pablogsal pablogsal pablogsal approved these changes

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bring back elapsed time debugging in gc.c

5 participants

@pgdr @sergey-miryanov @efimov-mikhail @nascheme @pablogsal