GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-128807: Add marking phase for free-threaded cyclic GC by nascheme · Pull Request #128808 · python/cpython

Conversation

@nascheme

@nascheme nascheme commented Jan 14, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This is conceptually similar to the phase that was added to the non-free-threaded GC. Start with a set of known root objects, like sysdict and mark all objects reachable from those (revealed by the tp_traverse method) as "alive". We know anything marked alive cannot be garbage and can be excluded from the regular cyclic GC process. For most programs, this saves a moderate amount of computation since the marking pass is relatively cheaper per object.

If gc.freeze() is used, it's unlikely that this marking phase will be a win since it's expected that the majority of objects will be frozen. Disable the marking phase if freeze is used.

See gh-126491 for the non-free-threaded version of this technique.

pyperformance results vs merge base. I suspect the slowdown on some benchmarks is not real. For example, regex_v8 should not be slower.

Here are the pyperformance results for a bare-metal AMD Ryzen machine. It does not show a slowdown on regex_v8, for example.

To better show the expected improvement, I ran a "sphinx build" benchmark, like in gh-124567. Results are:

old new
GC collections 38 38
long-lived objects 586,986 585,730
total run time 3.24 s 3.03 s
mark phase time 0.00 s 0.16 s
total gc time 0.72 s 0.57 s
  • Issue: Add marking phase to free-threaded cyclic GC #128807

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

corona10, thinkwelltwd, colesbury, mpage, AA-Turner, dolfinus, and sergey-miryanov reacted with hooray emoji
All reactions
  • 🎉 7 reactions
@nascheme
wip: gc mark alive for nogil
a111bff
@ghost

ghost commented Jan 14, 2025 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot mentioned this pull request Jan 14, 2025
Add marking phase to free-threaded cyclic GC #128807
Closed
@nascheme nascheme added topic-free-threading performance Performance or resource usage labels Jan 14, 2025
nascheme added 14 commits January 13, 2025 17:09
@nascheme
wip: mark more roots
200b69b
@nascheme
wip: log gc timing to temp file
713689a
@nascheme
wip: mark stack refs as alive
2635fe0
@nascheme
wip: include # of collected in debug log
72128e2
@nascheme
wip: move freeze_used to gc state
942f77d
@nascheme
wip: fix OOM error handling, add ifdef toggles
a3089e5
@nascheme
wip: fix reversion on mark_heap_visitor()
4f99de6
If the object is already marked as reachable, we shouldn't traverse it
again.
@nascheme
wip: revert changes to gc_should_collect()
4f6e539
These are not strictly needed, simplify PR.
@nascheme
wip: removing timing code
b99f2df
@nascheme
wip: code cleanup, small bug fix
5f6ab4c
More code cleanup (better names, comments, simplify error handling).
Fix bug in that "alive" bit must be checked in mark_alive_stack_push()
to avoid visiting already visited objects.
@nascheme
wip: untrack tuples in "mark alive" pass
bd46b5f
Make sure we still do this optimization.  There is also a unit test that
checks for this.
@nascheme
wip: enable stacks and extra roots
d25cb4a
@nascheme
wip: add helper gc_maybe_untrack()
347db45
Reduces duplicate code.
@nascheme
Add NEWS entry.
680f80f
@nascheme
nascheme force-pushed the nogil-gc-mark-alive branch from ccc5a11 to 680f80f Compare January 14, 2025 01:10
nascheme added 2 commits January 13, 2025 17:22
@nascheme
wip: spelling fixes, minor code cleanup
0c173c9
@nascheme
Merge branch 'origin/main' into nogil-gc-mark-alive
6654424
@nascheme
nascheme marked this pull request as ready for review January 14, 2025 18:12
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 14, 2025
colesbury
colesbury reviewed Jan 14, 2025
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

Looks great! A few comments below

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Python/gc_free_threading.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Python/gc_free_threading.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Python/gc_free_threading.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

nascheme added 3 commits January 14, 2025 14:45
@nascheme
wip: use -1 for error convention for functions
79ea47e
@nascheme
wip: improve error handling for OOM
0090365
Use gc_abort_mark_alive() helper in case of OOM.  In addition to
freeing the stack, we need to ensure that no object has the alive bit
set on it.  This also adding missing error handling in the case that
propagate_alive_bits() fails.
@nascheme
Add comment about gc.freeze() disabling marking.
6100691
kumaraditya303
kumaraditya303 reviewed Jan 15, 2025
View reviewed changes
Comment thread Python/gc_free_threading.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

colesbury
colesbury approved these changes Jan 15, 2025
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nascheme reacted with laugh emoji
All reactions
  • 😄 1 reaction
Comment thread Python/gc_free_threading.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Python/gc_free_threading.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jan 15, 2025
@nascheme
nascheme merged commit 080f444 into python:main Jan 15, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 15, 2025
@nascheme nascheme mentioned this pull request Apr 28, 2025
Incremental "mark alive" pass for cyclic GC #126511
Closed
@pablogsal pablogsal mentioned this pull request Oct 11, 2026
gh-159137: Restore known-root marking for the GIL garbage collector #159162
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@colesbury colesbury colesbury approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

performance Performance or resource usage topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@nascheme @colesbury @kumaraditya303
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.