RFR: 8256265: G1: Improve parallelism in regions that failed evacuation [v3]

Thomas Schatzl tschatzl at openjdk.org
Tue Sep 13 12:28:08 UTC 2022


On Tue, 13 Sep 2022 11:53:47 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   sjohanss, albert review
>
> src/hotspot/share/gc/g1/g1EvacFailureRegions.inline.hpp line 41:
> 
>> 39:     HeapRegion* hr = g1h->region_at(region_idx);
>> 40:     G1CollectorState* state = g1h->collector_state();
>> 41:     hr->note_self_forwarding_removal_start(state->in_concurrent_start_gc());
> 
> It's unclear to me why it's called *here* -- we are still in the evacuation phase, self-forwarding-removal occurs in post-evacuation, isn't it?

> void HeapRegion::note_self_forwarding_removal_end_par(size_t garbage_bytes) {
  Atomic::store(&_live_bytes, region_size - _garbage_bytes, ...);
  Atomic::add(&_garbage_bytes, garbage_bytes, memory_order_relaxed);
}

This comment seems to be duplicated, I'll answer in the other question.

> It's unclear to me why it's [`note_self_forwarding_removal_start`] called here -- we are still in the evacuation phase, self-forwarding-removal occurs in post-evacuation, isn't it?

`note_self_forwarding_removal_start` prepares for self forwarding removal which is unavoidable at that point for that region. Previously the call to this method during self forward removal was easily possible because one thread only every worked on a single region.
The original code had an extra phase that iterated over all regions just for that, which has been removed [here](https://github.com/openjdk/jdk/pull/7047/commits/1003c77cfebc3a72f409d2ebff41b8a74c3e4495) I think, and this call placed here because of lack of other really good options (that are more complicated).

I will rename the methods/comments.

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

PR: https://git.openjdk.org/jdk/pull/9980


More information about the hotspot-dev mailing list