RFR: 8276887: G1: Move precleaning to Concurrent Mark From Roots subphase [v3]

[Kim Barrett]

On Sat, 13 Nov 2021 09:36:15 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/referenceProcessor.cpp line 1076:
>> 
>>> 1074:   size_t ref_count_arr[4] = {};
>>> 1075: 
>>> 1076:   if (discovery_is_mt()) {
>> 
>> The old form of these loops called `yield->should_return()` for each queue.  That's been lost in this rewrite, and I'm not sure that's correct.
>
> It's still correct because `preclean_discovered_reflist` checkes for if-aborted while iterating the list. I could have written sth like the following if you think prompt abort is important.
> 
> 
>   bool is_aborted = preclean_discovered_reflist(...);
>   if (is_aborted) {
>     return;
>   }

It's hard to know for sure, but the description of YieldClosure makes me think that's not really the intended usage.  OTOH, this seems to be the only use of YieldClosure; maybe there were others in CMS?  I would prefer the `yield->should_return()` checks be retained.  Also, `preclean_discovered_reflist` might no longer need to return bool.

>> src/hotspot/share/gc/shared/referenceProcessor.cpp line 1096:
>> 
>>> 1094:   }
>>> 1095: 
>>> 1096:   uint worker_id = WorkerThread::current()->id();
>> 
>> This logging is potentially far more verbose than previously, since there will now be a line for each concurrent marking thread, rather than one line for the previously single-threaded precleaning.  We have mechanisms for collecting and reporting timing and work units for parallel activities that should be used here.
>
> The new logs are on `trace` level, just FYI. How about I address all logging related issues (from Thomas and Stefan as well) in a follow-up PR?

I know this is trace logging, but that doesn't make the verbosity or content good.  Consider a machine with a large amount of concurrency; one might be interested in any of the totals/min/max/avg, and having to extract that manually is going to be annoying.  I'm okay with it being a followup though.

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

PR: https://git.openjdk.java.net/jdk/pull/6327