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

[Jon Masamitsu]

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 <mailto: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/
>>>     <http://cr.openjdk.java.net/%7Ejmasa/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/
>     <http://cr.openjdk.java.net/%7Ejmasa/8152208/webrev.01/>
>
>     Jon
>
>>
>>     Thanks,
>>     Bengt
>>
>>>
>>>     Thanks.
>>>
>>>     Jon
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160321/a40bf6f4/attachment.htm>