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

[Stefan Johansson]

On Tue, 13 Sep 2022 10:44:10 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1EvacFailure.cpp line 108:
>> 
>>> 106: 
>>> 107: // Caches the currently accumulated number of garbage words found in this heap region.
>>> 108: // Avoids direct (frequent) atomic operations on the HeapRegion's garbage counter.
>> 
>> Just to be sure we need this, we have seen problems calling `note_self_forwarding_removal_end_par(...)` on return from `process_chunk(...)`?
>
> I think that's it. We expect few failed regions (1-2) with pinned regions, so many threads working on the same region at the same time. If you want we can drop the cache, I did not (re-)measure.

I would prefer to drop it if not needed. From the comment it sounds like the cache was created to avoid updating the global value for each "garbage area", but a middle road would be to update the global value after each chunk. How much work is it to re-measure?

>> src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp line 106:
>> 
>>> 104:   _gc_par_phases[MergePSS] = new WorkerDataArray<double>("MergePSS", "Merge Per-Thread State (ms):", max_gc_threads);
>>> 105:   _gc_par_phases[RestoreRetainedRegions] = new WorkerDataArray<double>("RestoreRetainedRegions", "Restore Retained Regions (ms):", max_gc_threads);
>>> 106:   _gc_par_phases[RemoveSelfForwardsInChunks] = new WorkerDataArray<double>("RemoveSelfForwardsInChunks", "Remove Self Forwards In Chunks (ms):", max_gc_threads);
>> 
>> Is the "In Chunks" part really interesting to users? We do mention chunks below so the info is there. 
>> 
>> And if we could come up with a more user friendly name than "Remove Self Forwards" that would probably be good as well. Some thing like "Clean/Fix Failed Regions" maybe.
>
> I can see your point about the "InChunks" postfix; however I would prefer to keep the phase meaningful wrt to what it does. Neither "Remove Self Forwards" or "Fix Failed Regions" helps end users in a big way, but the former helps at least us understand at a glance what this phase does without needing to translate "Fix Failed Regions" to "this phase removes the self forwards".

Sure, that's a fair point. Logging at this level requires some knowledge to be fully meaningful as well.

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

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