Our performance test numbers calculate standard deviation incorrectly: (wrong) RESULT PageLoad: svg: files: worldcup.svg= 2197.27248895 ms median= 2157.99593925 ms, stdev= 693.92660072 ms, min= 2127.62403488 ms, max= 2865.42391777 ms (right) RESULT PageLoad: svg: files: worldcup.svg= 2177.03914642 ms median= 2146.1250782 ms, stdev= 162.849482453 ms, min= 2105.28707504 ms, max= 2840.33107758 ms
Created attachment 166608 [details] Fix PerfTest standard deviation calculation.
Comment on attachment 166608 [details] Fix PerfTest standard deviation calculation. View in context: https://bugs.webkit.org/attachment.cgi?id=166608&action=review Could you also change the algorithm in runner.js? I've always wanted to fix that algorithm. > Tools/Scripts/webkitpy/performance_tests/perftest.py:236 > + 'stdev': math.sqrt(squareSum / (len(sorted_test_times) - 1)), So we're now computing the sample standard deviation as opposed to the sample deviation of the sample: http://en.wikipedia.org/wiki/Standard_deviation#Estimation You should explain that in the change log.
Comment on attachment 166608 [details] Fix PerfTest standard deviation calculation. View in context: https://bugs.webkit.org/attachment.cgi?id=166608&action=review > Tools/Scripts/webkitpy/performance_tests/perftest.py:227 > delta = time - mean > sweep = i + 1.0 > mean += delta / sweep We can rewrite this as mean += (time - mean) / sweep and eliminate delta now.
Created attachment 166621 [details] Update per reviewer comments
(In reply to comment #2) > (From update of attachment 166608 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166608&action=review > > Could you also change the algorithm in runner.js? I've always wanted to fix that algorithm. Done :) > > > Tools/Scripts/webkitpy/performance_tests/perftest.py:236 > > + 'stdev': math.sqrt(squareSum / (len(sorted_test_times) - 1)), > > So we're now computing the sample standard deviation as opposed to the sample deviation of the sample: > http://en.wikipedia.org/wiki/Standard_deviation#Estimation > > You should explain that in the change log. Done
(In reply to comment #3) > (From update of attachment 166608 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166608&action=review > > > Tools/Scripts/webkitpy/performance_tests/perftest.py:227 > > delta = time - mean > > sweep = i + 1.0 > > mean += delta / sweep > > We can rewrite this as > mean += (time - mean) / sweep > and eliminate delta now. I'm not sure it's quite that simple (we need to store the mean before and after the update). This isn't performance-sensitive code and I think the readability (copied directly from Knuth) is useful here for future statistics hackers.
Comment on attachment 166621 [details] Update per reviewer comments Clearing flags since patch doesn't apply. Let me rebaseline after rniwa's change today and reupload.
Created attachment 166623 [details] Rebaselined
Comment on attachment 166623 [details] Rebaselined Please include the URL to the algorithm you cited.
*** Bug 77401 has been marked as a duplicate of this bug. ***
Created attachment 166626 [details] Added link to algorithm in the ChangeLog
Comment on attachment 166626 [details] Added link to algorithm in the ChangeLog Clearing flags on attachment: 166626 Committed r130135: <http://trac.webkit.org/changeset/130135>
All reviewed patches have been landed. Closing bug.
(In reply to comment #12) > (From update of attachment 166626 [details]) > Clearing flags on attachment: 166626 > > Committed r130135: <http://trac.webkit.org/changeset/130135> FYI: It broke the perf tests again. See the bots for details.
Fix landed in http://trac.webkit.org/changeset/130181
(In reply to comment #15) > Fix landed in http://trac.webkit.org/changeset/130181 Should we also make this change in the python standard deviation method?
(In reply to comment #16) > (In reply to comment #15) > > Fix landed in http://trac.webkit.org/changeset/130181 > > Should we also make this change in the python standard deviation method? That's a good point. We should do that although PageLoadTest always gets 20 values.