Skip to content

gh-159437: Skip unnecessary GC finalizer scans and align functions - #159587

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

mfalco-oai wants to merge 2 commits 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.

The build also requests 64-byte function alignment for GIL and free-threaded non-debug builds on Linux (excluding Android) and macOS when the compiler supports it, except when optimizing for size. Small collector changes have moved otherwise unchanged traversal functions to different offsets within cache lines and this had negatively impacted performance. Aligning function entry points should make performance less dependent on those offsets.

Performance

gc_collect_self_lists times full collection of one million self-referential lists. Before is upstream; after is this PR. Both builds use -falign-functions=64.

CPU Before After Change (95% interval) Result
AMD 87.404 ms 79.538 ms −9.00% [−11.48%, −6.21%] Improvement
ARM 70.179 ms 64.645 ms −7.89% [−8.66%, −7.09%] Improvement

Benchmarks: python/pyperformance#496

@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.

@mfalco-oai mfalco-oai changed the title gh-159437: Skip unnecessary GC finalizer scans gh-159437: Skip unnecessary GC finalizer scans and align functions Oct 19, 2026
@mfalco-oai
mfalco-oai force-pushed the mfalco-oai/gh-159437-finalizer-scans branch from 4a2dcac to ff1acf0 Compare October 21, 2026 13:23
@nascheme

Copy link
Copy Markdown
Member

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

I agree that the handle_callbacks() is a code style to be avoided. E.g. the argument list for the function is almost the same size as the body. Something wrong there. It's also only called once. So I'd say it should rather just be open-coded at the call site.

The idea of this optimization good. I'd suggest looking at Pablo's earlier PR and see if a more minimal code change can achieve the same thing.

The 64-bit alignment change really should be a separate PR. Imagine we want to revert one thing or the other. Should not be combined.

@nascheme

Copy link
Copy Markdown
Member

@pablogsal is your PR this one: #160464 ? Your link goes to a blob so it's a bit hard to find the commit that made it.

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.

3 participants