Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
GH-126491: GC: Mark objects reachable from roots before doing cycle collection#126502
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
iritkatriel left a comment
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.
Could you updatehttps://github.com/python/cpython/blob/main/InternalDocs/garbage_collector.md to explain how this works?
When you're done making the requested changes, leave the comment: |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
| } | ||
| } | ||
| if (!start&&frame->visited) { | ||
| // If this frame has already been visited, then the lower frames |
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.
There's a warning here
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.
It is a false positive. I think it is complaining because we don't setvisitedhere, but this line is unreachable for entry frames.
I'll initializevisited to keep it quiet and in case the code gets reordered.
b0fcc2c intopython:mainUh oh!
There was an error while loading.Please reload this page.
hugovk commentedNov 18, 2024
Hmm, looks like this commitb0fcc2c has caused
Reminder the 3.14 alpha 2 is due tomorrow (2024-11-19):https://buildbot.python.org/#/release_status |
nascheme commentedNov 18, 2024
Extracted from the Android failure: |
hugovk commentedNov 18, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Also some tier 1 and 2 "test_import failed (reference leak)" that include this change (and some others) are now failing:
Plus s390x RHEL8 + RHEL9 and AMD64 FreeBSD refleaks buildbots. I'll open a draft revert PR so we can run the buildbots on it. Edit:#126983 |
… doing cycle collection (pythonGH-126502)"This reverts commitb0fcc2c.
markshannon commentedNov 18, 2024
I don't have ready access to an iOS or Android device to test on. |
freakboy3742 commentedNov 18, 2024
I've done some local testing on iOS. The test_gc module passes as-is if you run it by itself. If I increase the loop count in the failing test to 100k, the test fails with exactly the same object count (25,032). My guess is that this one of those issues that iOS/Android expose because they don't run the test suite in parallel - my guess is that you'd likely be able to reproduce this failure on a desktop machine if you run the test suite with On that basis, following@markshannon's suggestion I'll increase the object count to 26k. PR incoming. |
freakboy3742 commentedNov 18, 2024
#126984 is a PR increasing the test threshold. |
freakboy3742 commentedNov 19, 2024
I've abandoned the "increase the threshold" aapproach - that's clearly not enough to avoid the issue. The threshold needed to pass the test is clearlymuch higher than 30k. The good news (well... good for me, not so good for@markshannon 😄 ) is that I can reproduce this bug on macOS. If you run the test suite sequentially in a single process ( Youdon't see this error if you run |
freakboy3742 commentedNov 19, 2024
I'm able to reliably reproduce the failure on macOS with |
hugovk commentedNov 19, 2024
I've reverted this because it's release day and we need to keep tier-1 green anyway:#126983. Let's make sure to summon the buildbots next time :) |
nascheme commentedNov 19, 2024
I'm able to re-produce on Linux with this set of tests: |
…ycle collection (pythonGH-126502)* Mark almost all reachable objects before doing collection phase* Add stats for objects marked* Visit new frames before each increment* Remove lazy dict tracking* Update docs* Clearer calculation of work to do.
… doing cycle collection (pythonGH-126502)" (python#126983)
Uh oh!
There was an error while loading.Please reload this page.
This PR:
Performance is excellent. Speedup is 4%, with a 100% speedup on one GC benchmark.
Stats show the GC is run more frequently and collects more objects, but does only ~60% of the work.
This PR also: