FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-137110: Untrack immortal objects from expand_region_transitivity_reachable by sergey-miryanov · Pull Request #137111 · python/cpython · GitHub

Repository navigation

gh-137110: Untrack immortal objects from expand_region_transitivity_reachable - #137111

Closed
sergey-miryanov wants to merge 1 commit into
python:mainfrom
sergey-miryanov:untrack-immortal-in-gc
Closed

sergey-miryanov wants to merge 1 commit into
python:mainfrom
sergey-miryanov:untrack-immortal-in-gc

Conversation

sergey-miryanov commented Jul 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

There is some inconsistency in handling of immortal objects in default-build's GC.

In update_refs immortal objects just untracked (all immortal objects that we face in GC is an accidentally immortalized).

cpython/Python/gc.c

Lines 495 to 500 in 1e69cd1

if (_Py_IsImmortal(op)) {
assert(!_Py_IsStaticImmortal(op));
_PyObject_GC_UNTRACK(op);
gc = next;
continue;
}

update_refs called for all collections (young, increment and full). But for incremental collection if increment_size < gcstate->work_to_do we steal some objects from neighbor gen and call expand_region_transitively_reachable where immortal objects moved to permanent generation:

cpython/Python/gc.c

Lines 1397 to 1402 in 1e69cd1

if (_Py_IsImmortal(op)) {
PyGC_Head *next = GC_NEXT(gc);
gc_list_move(gc, &gcstate->permanent_generation.head);
gc = next;
continue;
}

So, one part of immortal objects can be untracked, and other part can be moved to perm gen.

As I understand from #127519 it is preferring to untrack immortal objects in all cases.

CPython versions tested on:

CPython main branch

Copy link
Copy Markdown
Contributor Author

I'm not sure, but this probably should skip news.

Copy link
Copy Markdown
Contributor Author

AFAIU moving immortal objects to the permanent_generation is against the completed_scavenge logic, and the visited_space will stick in this case.

Copy link
Copy Markdown
Contributor Author

@nascheme Could you please take a look?

efimov-mikhail 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown
Member

@nascheme Could you please take a look?

I can (although not just now). Probably @markshannon would be the most useful reviewer since he's most familiar with that code.

Copy link
Copy Markdown
Member

moving immortal objects to the permanent_generation is against the completed_scavenge logic, and the visited_space will stick in this case.

I'd need to study the GC code more to familiarize myself with how this works. However, would it be possible to change the other case and always move immortal objects to the permanent generation, rather than untracking? I think it's safer to do that, for a couple of reasons. First, the 3.13 GC did not untrack, only move. Second, if an object's dealloc assumes that it is tracked, untracking it could cause a crash (if _PyObject_GC_UNTRACK is used). I don't know if that can actually happens but it seems like a risk to me. Finally, gc.get_objects() will no longer return this immortal objects. Again, I don't know if that's actually a problem (people probably shouldn't be making strong assumptions about what that function returns) but it seems safer to not untrack.

I would guess that immortal container objects (tracked by the GC) are quite rare. So the performance benefit from untracking should be tiny.

Copy link
Copy Markdown
Contributor Author

Thanks!

We always untrack newly created immortal objects. In the PR that I linked (which unfortunately was reverted), immortal objects were untracked in an analogous function - https://github.com/python/cpython/pull/127519/files#diff-a848a0ef178aa113a092e72403da0e344f37bd141bb90a7aa65015c77bfe7385R1451.

Also, IIUC, moving immortal objects to the permanent generation would prevent us from flipping visited and pending spaces unless we remove those objects from the permanent generation.

OTOH, the case that handled by this condition is very rare - it is for accidentally immortalized objects.

Copy link
Copy Markdown
Contributor

However, would it be possible to change the other case and always move immortal objects to the permanent generation, rather than untracking? I think it's safer to do that, for a couple of reasons. First, the 3.13 GC did not untrack, only move. Second, if an object's dealloc assumes that it is tracked, untracking it could cause a crash (if _PyObject_GC_UNTRACK is used).

The free-threading gc untracks any immortal objects encountered during gc so I think it makes sense to do the same for normal builds as well.

Copy link
Copy Markdown
Contributor Author

@markshannon Could you please take a look?

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

Now that the generational GC has landed, I'll close this as obsolete.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review skip news stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL