|Summary:||Dromaeo perf-tests results are wrong|
|Product:||WebKit||Reporter:||Kentaro Hara <haraken>|
|Component:||Tools / Tests||Assignee:||Kentaro Hara <haraken>|
|Severity:||Normal||CC:||abarth, benjamin, morrita, rniwa, webkit.review.bot|
|Version:||528+ (Nightly build)|
|Bug Depends on:||81142|
Description Kentaro Hara 2012-03-13 01:33:07 PDT
Regarding Dromaeo/dom-attr.html, the ./run-perf-test result is as follows: RESULT Dromaeo: dom-attr= 6270.733871 ms median= 0.0 ms, stdev= 38.4424019328 ms, min= 6203.83270023 ms, max= 6354.54507606 ms Finished: 30.665235 s When I manually change the loop count from 10240 to 1, the ./run-perf-test result becomes: RESULT Dromaeo: dom-attr= 21839544.4 ms median= 0.0 ms, stdev= 50594.7730808 ms, min= 21685174.0 ms, max= 21970762.0 ms Finished: 30.582642 s As you can see, the "ms" number does not make sense. The number should represent the execution time per iteration and thus should be "smaller is better". Also, "median" should not be "0.0 ms".
Comment 2 Hajime Morrita 2012-03-13 02:42:53 PDT
Comment on attachment 131576 [details] Patch Please consider to keep using "run/s" since it's stable. I tried ms. But making them resulted unexpected (and typically longer) running time, especially we cannot assume underlying environment. Also, I guess |runStyle| could be specified externally via URL query string. Please do it if possible. Since this Dromaeo source is copied from upstream. If there is any obvious errors there, I encourage you to upstream the fix.
Comment 3 Kentaro Hara 2012-03-13 02:52:44 PDT
(In reply to comment #2) > (From update of attachment 131576 [details]) > Please consider to keep using "run/s" since it's stable. > I tried ms. But making them resulted unexpected (and typically longer) running time, especially we cannot assume underlying environment. This is the reason why I removed 'if (runStyle === "runs/s")' part. Now the running time does not change between run/s and ms. > Also, I guess |runStyle| could be specified externally via URL query string. > Please do it if possible. Let me try it. > Since this Dromaeo source is copied from upstream. > If there is any obvious errors there, > I encourage you to upstream the fix. If the patch looks OK, I'll try it. But it will take time, and I would like to land the patch into WebKit first. The current Dromaeo results have been preventing me from evaluating performance optimization patches.
Comment 7 WebKit Review Bot 2012-03-13 07:10:08 PDT
Comment on attachment 131600 [details] Patch Clearing flags on attachment: 131600 Committed r110559: <http://trac.webkit.org/changeset/110559>
Comment 8 WebKit Review Bot 2012-03-13 07:10:14 PDT
All reviewed patches have been landed. Closing bug.
Comment 9 Benjamin Poulain 2012-03-14 12:18:43 PDT
I think this broke most of the Dromaeo tests on WebKit-perf: http://webkit-perf.appspot.com/ They all report a time of 0 now.
Comment 10 Ryosuke Niwa 2012-03-14 12:21:26 PDT
Comment on attachment 131600 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131600&action=review > PerformanceTests/Dromaeo/resources/dromaeorunner.js:16 > + median += 1000 / item.median; This makes no sense. The sum of medians isn't the median of all samples.