Bug 80974

Summary: Dromaeo perf-tests results are wrong
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: Tools / TestsAssignee: Kentaro Hara <haraken>
Status: RESOLVED INVALID    
Severity: Normal CC: abarth, benjamin, morrita, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 81142    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Kentaro Hara
Reported 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".
Attachments
Patch (4.05 KB, patch)
2012-03-13 02:32 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (2.39 KB, patch)
2012-03-13 03:21 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (2.51 KB, patch)
2012-03-13 06:22 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (2.45 KB, patch)
2012-03-13 06:29 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2012-03-13 02:32:50 PDT
Created attachment 131576 [details] Patch
Hajime Morrita
Comment 2 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.
Kentaro Hara
Comment 3 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.
Kentaro Hara
Comment 4 2012-03-13 03:21:24 PDT
Created attachment 131582 [details] Patch
Kentaro Hara
Comment 5 2012-03-13 06:22:44 PDT
Created attachment 131599 [details] Patch
Kentaro Hara
Comment 6 2012-03-13 06:29:16 PDT
Created attachment 131600 [details] Patch
WebKit Review Bot
Comment 7 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>
WebKit Review Bot
Comment 8 2012-03-13 07:10:14 PDT
All reviewed patches have been landed. Closing bug.
Benjamin Poulain
Comment 9 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.
Ryosuke Niwa
Comment 10 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.
Ryosuke Niwa
Comment 11 2012-03-14 12:24:19 PDT
What we need to do is to fix https://bugs.webkit.org/show_bug.cgi?id=78303. We don't want to invent our own score for Dromaeo.
Note You need to log in before you can comment on or make changes to this bug.