RFR: 8320525: G1: G1UpdateRemSetTrackingBeforeRebuild::distribute_marked_bytes accesses partially unloaded klass

[Albert Mingkun Yang]

On Fri, 24 Nov 2023 08:39:30 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> We need to call `HeapRegion::note_end_of_marking` on them, even if they are empty for completeness. It's not completely necessary because reclamation will probably reset them correctly, but it's easier to reason if they (empty and nonempty regions) are handled the same to me.
>> 
>> I.e. so _all_ regions have `note_start/end_of_marking` called.
>
> I do not think it is easier to understand to separate these two cases (empty/non-empty) regions here: both distributing bytes (it's fine to distribute 0 bytes) and being consistent with calling the start/end notifications (and `note_end` works as expected on empty regions too) for all regions is easier to follow to me compared to having an unnecessary exception.
> Because then the question is: why have that exception?
> 
> Even if they do not do anything "meaningful" other than resetting some internal state that is later overwritten for these specially handled empty regions anyway.

> so all regions have note_start/end_of_marking called.

Currently, the pair is invoked on all regions and the filtering (skipping young-region for example) is done inside these methods. However, the logic why a particular kind of region can (should) be skipped really belongs to the caller. The region itself doesn't know how to react to marking-start/end. (This is kind of tied to the ticket of moving marking-related fields outside region.)

> why have that exception?

Because live-region and effective-region are diff, and mixing them causes confusion. I think the existence of the new comment "we should not access their header any more them..." demonstrates that it's not super obvious why the current code (before this PR) is problematic.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16766#discussion_r1404318347