Request for Review - 8152208: Summary for phase times are incorrect with and without UseDynamicNumberOfGCThreads
jon.masamitsu at oracle.com
Mon Mar 21 23:05:21 UTC 2016
I've updated the 01 version. The test has not changed.
Thanks again for catching that.
On 03/21/2016 03:16 PM, Tao Mao wrote:
> This change has a bug: double counting get(0); should start with s = 0
> 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:
> 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
>> 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
> webrev. The test has not changed.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev