Repository navigation
gh-159437: Skip unnecessary late weakref cleanup - #159590
mfalco-oai wants to merge 3 commits into
Conversation
Count the stable unreachable set while preparing callbacks and retain a conservative cleanup flag. Reuse the count only when late weakref clearing has no work, and force fresh cleanup and counting after resurrection analysis. Add a regression for a callback-free weakref observed when clearing a young cycle releases an older finalizer.
|
All commit authors signed the Contributor License Agreement. |
There was a problem hiding this comment.
Let's keep the counting change and hold off on this part until the application benefit is clearer. That already removes the separate counting pass. This adds a count before callbacks, then rules for deciding whether to reuse it or count everything again.
I tried parsing SQL with SQLglot, parsing Python with Parso and running Pylint, comparing this change with that version. There was quite a bit of noise and no clear application win. That leaves the extra cached count and cleanup state to maintain without enough evidence yet that they pay off.
| if (! _PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) { | ||
| // Garbage weakrefs need clearing even if their referents are outside | ||
| // this collection (bpo-38006). | ||
| needs_weakref_cleanup |= PyWeakref_Check(op); |
There was a problem hiding this comment.
btw, |= keeps calling PyWeakref_Check() after needs_weakref_cleanup is already true. We could stop checking at that point. Would need measuring whether the extra branch helps though.
2 participants
This is stacked on the preceding counting change. That change counts collectable objects during
clear_weakrefs()and removes the separategc_list_size()walk. It still runsclear_weakrefs()for every collection.With this optimization the GIL collector can skip that late cleanup pass. Callback preparation already visits the collectable objects, so it also records their count and whether weakrefs may need cleanup. If that scan finds no weakrefs that may need clearing and no Python code could have changed the garbage set, the collector reuses the count and skips
clear_weakrefs().When cleanup is needed, the collector uses the count returned by
clear_weakrefs(), as in the preceding change. A resurrection check always takes this path because Python code may have changed the garbage set or created new weakrefs.