<div dir="ltr">This change has a bug: double counting get(0); should start with s = 0<div><br></div><div>Thanks.</div><div>Tao Mao</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 21, 2016 at 2:55 PM, Jon Masamitsu <span dir="ltr"><<a href="mailto:jon.masamitsu@oracle.com" target="_blank">jon.masamitsu@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    <font face="Times New Roman, Times, serif">Bengt,<br>
      <br>
      Thanks for the review.<br>
    </font><span class=""><br>
    <div>On 03/21/2016 02:13 AM, Bengt Rutisson
      wrote:<br>
    </div>
    <blockquote type="cite">
      <br>
      Hi Jon,
      <br>
      <br>
      On 2016-03-21 03:43, Jon Masamitsu wrote:
      <br>
      <blockquote type="cite">The averages reported for phase times (for
        example "Ext Root Scanning") were
        <br>
        incorrect.  Not all the per thread values were included in the
        sum and the
        <br>
        average value was incorrect (this with build 9-ea+1100)
        <br>
        <br>
        [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
        <br>
        [0.366s][trace][gc,phases,task       ]
        GC(2)                                0.4  0.3
        <br>
        <br>
        With the fix all values are included in the sum and the average
        is correct.
        <br>
        <br>
        [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
        <br>
        [2.830s][trace][gc,phases,task       ]
        GC(0)                                8.9  5.7
        <br>
        <br>
        <a href="https://bugs.openjdk.java.net/browse/JDK-8152208" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8152208</a>
        <br>
        <a href="http://cr.openjdk.java.net/~jmasa/8152208/webrev.00/" target="_blank">http://cr.openjdk.java.net/~jmasa/8152208/webrev.00/</a>
        <br>
      </blockquote>
      <br>
      Nice catch! Your change looks good.
      <br>
      <br>
      The method WorkerDataArray<T>::sum(uint active_threads) just
      above the average() method has the same issue. Can you fix that
      too?
      <br>
    </blockquote>
    <br></span>
    Yes, indeed.<br>
    <br>
    I messed up the delta a bit so all the changes are in the
    workerDataArray.inline.hpp<br>
    webrev.  The test has not changed.<br>
    <br>
    <a href="http://cr.openjdk.java.net/~jmasa/8152208/webrev.01/" target="_blank">http://cr.openjdk.java.net/~jmasa/8152208/webrev.01/</a><br>
    <br>
    Jon<br>
    <br>
    <blockquote type="cite">
      <br>
      Thanks,
      <br>
      Bengt
      <br>
      <br>
      <blockquote type="cite">
        <br>
        Thanks.
        <br>
        <br>
        Jon
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </div>

</blockquote></div><br></div>