Skip to content

gh-159437: Skip unnecessary GC finalizer scans - #159587

Draft
mfalco-oai wants to merge 1 commit into
python:mainfrom
mfalco-oai:mfalco-oai/gh-159437-finalizer-scans
Draft

mfalco-oai wants to merge 1 commit into
python:mainfrom
mfalco-oai:mfalco-oai/gh-159437-finalizer-scans

Conversation

@mfalco-oai

@mfalco-oai mfalco-oai commented Oct 16, 2026 •

Copy link
Copy Markdown

With this optimization the GIL collector records pending finalizers while preparing weakref callbacks. If neither finalizers nor callbacks are pending, it skips the finalizer scan. Callbacks and DEBUG_COLLECTABLE keep the full path because they can run Python.

@python-cla-bot

python-cla-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 16, 2026

@pablogsal pablogsal left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callback refactor seems bigger than skipping this scan needs. There is a new helper with a long argument list and the caller now has to choose between different garbage lists.

A related change I made in an earlier PR already tracks whether Python may have run. I tried extending that flow by recording pending finalizers during the weakref scan. That was a much smaller change in gc.c and kept the existing callback handling.

Let's keep the existing callback and resurrection flow unless there is a separate application benefit from the larger refactor.

Let's see what @nascheme thinks about the callback refactor too.

Comment thread Lib/test/test_gc.py
return len(text)

writer = Stderr()
gc.collect()

@pablogsal pablogsal Oct 17, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails on its own with -X lazy_imports=all. I tried it with and without the PR and hit the collected object count assertion in each case. Binding patch_stderr = unittest.mock.patch.object before the initial collection fixed the failure. We should do that before this cleanup and use the bound function below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants