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