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

[Albert Mingkun Yang]

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

>> src/hotspot/share/gc/g1/g1ParScanThreadState.cpp line 632:
>> 
>>> 630:     // Mark the failing object in the marking bitmap and later use the bitmap to handle
>>> 631:     // evacuation failure recovery.
>>> 632:     _g1h->mark_evac_failure_object(_worker_id, old, word_sz);
>> 
>> The same live-size is calculated again in `process_chunk`, isn't it?
>
> You mean the one (ultimately) recorded in the `G1ConcurrentMark::_live_bytes` array and the one for the statistics in `G1RemoveSelfForwardsInChunksTask::process_chunk`?
> You probably suggest to merge them somehow to do this work once too?
> 
> I do not think this is worth, particularly because `process_chunk` only collects live bytes per thread without any synchronization (it does collect dead bytes with synchronization).
> So you could use that value (the total dead bytes) or vice versa to update `G1ConcurrentMark::_live_bytes` at the end of collection, but
> * `G1ConcurrentMark::_live_bytes` (the sum) isn't actually gathered here, but again these are basically per-thread values (using the mark-stats cache), so otoh the update is inexpensive (nothing compared to the `par_mark` in `mark_evac_failure_object`), and otoh it can't be used for updating `HeapRegion::_garbage_bytes` directly. This would need to flush buffers somewhere before doing that.
> * the collected `HeapRegion::_garbage_bytes` could be used at the end to update `G1ConcurrentMark::_live_bytes`, but it does not seem to be worth the complexity adding another phase somewhere else. Also it would require another `G1ConcurrentMark` API entry just for that which I do not see worth doing.
> 
> I.e. I do not think it is worth optimizing something that trivial and add the required code complexity. I would prefer to decrease code complexity.

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);
}

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

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