Request for Review - 8152208: Summary for phase times are incorrect with and without UseDynamicNumberOfGCThreads

Tao Mao yiyeguhu at gmail.com
Tue Mar 22 03:06:51 UTC 2016


Looks good again. Thanks. -Tao

On Mon, Mar 21, 2016 at 4:05 PM, Jon Masamitsu <jon.masamitsu at oracle.com>
wrote:

> Tao,
>
> I've updated the 01 version.  The test has not changed.
> Thanks again for catching that.
>
> Jon
>
> On 03/21/2016 03:16 PM, Tao Mao wrote:
>
> This change has a bug: double counting get(0); should start with s = 0
>
> Thanks.
> Tao Mao
>
> On Mon, Mar 21, 2016 at 2:55 PM, Jon Masamitsu <jon.masamitsu at oracle.com>
> wrote:
>
>> Bengt,
>>
>> Thanks for the review.
>>
>> On 03/21/2016 02:13 AM, Bengt Rutisson wrote:
>>
>>
>> Hi Jon,
>>
>> On 2016-03-21 03:43, Jon Masamitsu wrote:
>>
>> The averages reported for phase times (for example "Ext Root Scanning")
>> were
>> incorrect.  Not all the per thread values were included in the sum and
>> the
>> average value was incorrect (this with build 9-ea+1100)
>>
>> [0.366s][debug][gc,phases            ] GC(2)     Ext Root Scanning
>> (ms):   Min:  0.3, Avg:  0.2, Max:  0.4, Diff:  0.0, Sum:  0.3
>> [0.366s][trace][gc,phases,task       ]
>> GC(2)                                0.4  0.3
>>
>> With the fix all values are included in the sum and the average is
>> correct.
>>
>> [2.830s][debug][gc,phases            ] GC(0)     Ext Root Scanning
>> (ms):   Min:  5.7, Avg:  7.3, Max:  8.9, Diff:  3.1, Sum: 14.6
>> [2.830s][trace][gc,phases,task       ]
>> GC(0)                                8.9  5.7
>>
>> https://bugs.openjdk.java.net/browse/JDK-8152208
>> http://cr.openjdk.java.net/~jmasa/8152208/webrev.00/
>>
>>
>> Nice catch! Your change looks good.
>>
>> The method WorkerDataArray<T>::sum(uint active_threads) just above the
>> average() method has the same issue. Can you fix that too?
>>
>>
>> Yes, indeed.
>>
>> I messed up the delta a bit so all the changes are in the
>> workerDataArray.inline.hpp
>> webrev.  The test has not changed.
>>
>> http://cr.openjdk.java.net/~jmasa/8152208/webrev.01/
>>
>> Jon
>>
>>
>> Thanks,
>> Bengt
>>
>>
>> Thanks.
>>
>> Jon
>>
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20160321/875f22bb/attachment.html>


More information about the hotspot-gc-dev mailing list