RFR: 8252141: Rename G1YoungRemSetSamplingThread to better reflect its purpose
stefan.johansson at oracle.com
stefan.johansson at oracle.com
Tue Sep 1 10:50:45 UTC 2020
Hi Thomas,
On 2020-08-31 15:20, Thomas Schatzl wrote:
> Hi,
>
> On 26.08.20 09:18, stefan.johansson at oracle.com wrote:
>> Hi Kim,
>>
>> Thanks for reviewing.
>>
>> On 2020-08-25 04:44, Kim Barrett wrote:
>>>> On Aug 24, 2020, at 5:36 PM, stefan.johansson at oracle.com wrote:
>>>>
>>>> Hi,
>>>>
>>>> Please review this rename of the G1 Young RemSet Sampling thread.
>>>> [...]
>> Nice catch, updated in:
>> Full: http://cr.openjdk.java.net/~sjohanss/8252141/01/
>> Inc: http://cr.openjdk.java.net/~sjohanss/8252141/00-01/
>>
>> I also filed a issue to take care of the name at some point:
>> https://bugs.openjdk.java.net/browse/JDK-8252358
>
> Thanks.
>
Thanks for reviewing.
> Some comments:
>
> - g1RemSetSummary.cpp:330:
>
> out->print_cr(" Concurrent sampling threads times (s)");
> out->print_cr(" %5.2f", service_thread_vtime());
>
> should probably be "Service thread time (s)".
>
I was thinking about changing this but wasn't sure, since this is for
the rem-sets. I think going forward maybe we want to change how we
measure the time to only account the actual time spent on the remsets.
Filed an issue for this:
https://bugs.openjdk.java.net/browse/JDK-8252645
In the mean time I updated the code per your request.
> - if you are bored, some copyright dates need updates
>
I'm not bored, but since you called it out :)
> - pre-existing: maybe the includes in G1ServiceThread.hpp could be
> fixed; at least runtime/mutex.hpp should be included for Monitor.
>
I added it and os.hpp.
Updated webrevs:
Full: http://cr.openjdk.java.net/~sjohanss/8252141/02
Inc: http://cr.openjdk.java.net/~sjohanss/8252141/01-02
Thanks,
Stefan
> Thanks,
> Thomas
More information about the hotspot-gc-dev
mailing list