gh-137110: Untrack immortal objects from expand_region_transitivity_reachable

[sergey-miryanov]

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

• Issue: Inconsistency handling of immortal objects in gc #137110

[sergey-miryanov]

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

[sergey-miryanov]

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

[sergey-miryanov]

@nascheme Could you please take a look?

[efimov-mikhail]

LGTM

[nascheme]

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

[nascheme]

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.

[sergey-miryanov]

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.

[kumaraditya303]

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.

[sergey-miryanov]

@markshannon Could you please take a look?