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

[Albert Mingkun Yang]

On Tue, 13 Sep 2022 12:38:35 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> I was thinking sth like:
>> 
>> 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);
>> }
>
> Live-bytes are required to be updated for the marking, i.e. within the concurrent start pause. `HeapRegion` never had `_live_bytes`, only `_marked_bytes` which you probably meant.
> 
> `_marked_bytes` has been removed in  [JDK-8290357](https://bugs.openjdk.org/browse/JDK-8290357) because it's not required to actually store that value. It has only ever been used for some unnecessary reasons (see my comments in https://github.com/openjdk/jdk/pull/9511 for the removal reasons). 
> 
> At the end of marking the marked bytes (or actually `HeapRegion::_garbage_bytes`) are set by the `Remark` pause based on its `live/marked_bytes`. So we need to update the marking's `_live_bytes` here so that the values add up in the end (without using extra storage somewhere).
> 
> Actually I think now that we do not strictly need to update `HeapRegion::_garbage_bytes` in evacuation failure handling; updating the value for the marking is sufficient as we only use it (apart from some logging) for determining the region's gc efficiency. That one is strictly using the information from the latest marking (just completed), so that update isn't really necessary. I can remove this if wanted, but if so I'd prefer to do that separately.

I see; I was assuming `_live_bytes` and `_garbage_bytes` live in the same class/context/entity. In current impl, `_live_bytes` lives in CM while `_garbage_bytes` in `HeapRegion`, which is quite confusing IMO.

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

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