RFR (M): 6672778: G1 should trim task queues more aggressively during evacuation pauses

[sangheon.kim]

Hi Thomas,

This is nice improvement, thank you for doing this.

On 04/25/2018 04:13 AM, Thomas Schatzl wrote:
> Hi,
>
>    thanks for your review, Stefan.
>
> Could I have another reviewer for this change?
>
> Thanks,
>    Thomas
>
> On Thu, 2018-04-19 at 11:27 +0200, Stefan Johansson wrote:
>> On 2018-04-19 10:09, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     I unfortunately found another issue with timing:
>>>
>>> when calculating the "Other" time, we add the SATBFiltering phase
>>> to the known worker time - however that time is already included in
>>> the ext root scan time, so it is double-counted, and you
>>> occasionally get slightly negative "Other" times.
>>>
>>> This problem had been introduced in the refactoring too :(
>>>
>>> This change fixes that.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/6672778/webrev.3_to_4/ (diff)
>>> http://cr.openjdk.java.net/~tschatzl/6672778/webrev.4/ (full)
webrev.4 looks good to me.

If you are interested, please add 'const' before pushing this.

src/hotspot/share/gc/g1/g1ParScanThreadState.hpp
  204   Tickspan trim_ticks();

src/hotspot/share/gc/g1/g1RemSet.hpp
  174   Tickspan strong_code_root_scan_time() { return 
_strong_code_root_scan_time;  }
  175   Tickspan strong_code_root_trim_partially_time() { return 
_strong_code_trim_partially_time; }

Thanks,
Sangheon


>> Good that you caught that! Still good.
>>
>> Thanks,
>> Stefan
>>
>>> Thanks,
>>>     Thomas
>>>
>>> On Mon, 2018-04-16 at 14:58 +0200, Stefan Johansson wrote:
>>>> Hi Thomas,
>>>>
>>>> On 2018-04-16 13:08, Thomas Schatzl wrote:
>>>>> Hi all,
>>>>>
>>>>> On Fri, 2018-04-13 at 14:25 +0200, Stefan Johansson wrote:
>>>>>> On 2018-04-13 10:35, Thomas Schatzl wrote:
>>>>>>> Hi Stefan,
>>>>>>>
>>>>>>>       thanks for your review... :)
>>>>>>>
>>>>>>> On Thu, 2018-04-12 at 17:15 +0200, Stefan Johansson wrote:
>>>>>>>> Hi Thomas,
>>>>>>>>
>>>>>>>> On 2018-04-11 13:46, Thomas Schatzl wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>>        I updated and (hopefully) improved the change a
>>>>>>>>> bit
>>>>>>>>> after
>>>>>>>>> some
>>>>>>>>> more thinking.
>>>>>>> [...]
>>>>> Also fixed a problem with the "-" operator of Tickspan
>>>>> introduced
>>>>> in
>>>>> all this refactoring. This caused negative times being reported
>>>>> sometimes in the logs.
>>>>>
>>>>> http://cr.openjdk.java.net/~tschatzl/6672778/webrev.2_to_3/
>>>>> (diff)
>>>>> http://cr.openjdk.java.net/~tschatzl/6672778/webrev.3/ (full)
>>>>>
>>>>> The change looks really nice now imho, thanks Stefan!
>>>> I agree, ship it :)
>>>>
>>>> Thanks,
>>>> Stefan
>>>>> Thanks,
>>>>>      Thomas
>>>>>