Bug 94668

Summary: Perf test results is incomprehensive
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Tools / TestsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, arv, eae, eric, haraken, leviw, mjs, morrita, ojan, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77037    
Attachments:
Description Flags
tabular view
none
New tabular view
none
Improved some cosmetics and hides memory results
none
Added Time/Memory toggle button
none
Got rid of scientific notation and added the toggle button for reference run
none
Fixes the bug
none
Sample output none

Ryosuke Niwa
Reported 2012-08-21 21:07:35 PDT
Having to go through through hundreds to verify whether a patch regressed or progressed performance isn't practical. We need a better way of looking at perf test results.
Attachments
tabular view (95.12 KB, text/html)
2012-08-21 21:09 PDT, Ryosuke Niwa 		no flags
Details
New tabular view (136.31 KB, text/html)
2012-08-24 02:14 PDT, Ryosuke Niwa 		no flags
Details
Improved some cosmetics and hides memory results (137.21 KB, text/html)
2012-08-24 03:22 PDT, Ryosuke Niwa 		no flags
Details
Added Time/Memory toggle button (137.44 KB, text/html)
2012-08-24 17:03 PDT, Ryosuke Niwa 		no flags
Details
Got rid of scientific notation and added the toggle button for reference run (138.80 KB, text/html)
2012-08-25 02:34 PDT, Ryosuke Niwa 		no flags
Details
Fixes the bug (36.34 KB, patch)
2012-09-14 15:38 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Sample output (139.54 KB, text/html)
2012-09-14 15:40 PDT, Ryosuke Niwa 		no flags
Details
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-08-21 21:09:22 PDT
Created attachment 159854 [details] tabular view Here's a sample output from perfalizer: https://bug-93629-attachments.webkit.org/attachment.cgi?id=158183 And the attachment is an experimental tabular view of the same data.
Ryosuke Niwa
Comment 2 2012-08-21 21:10:32 PDT
Better & Worse is computed from the percentage difference between the two. We report better/worse if they're better than the standard deviation of either sample.
Zoltan Horvath
Comment 3 2012-08-22 01:18:54 PDT
Cool! Both the perfalizer and the tabular view compare the same revision. I think it would be useful to provide option to compare a range of revisions (avg of them) with a revision. Hmm... It would be also good if we could provide an overall/summarized result: I mean something like: From 100 FastMalloc results 69 were better 31 were worse What is your opinion?
Ryosuke Niwa
Comment 4 2012-08-24 02:12:24 PDT
(In reply to comment #3) > Both the perfalizer and the tabular view compare the same revision. > > I think it would be useful to provide option to compare a range of revisions (avg of them) with a revision. There is a way already! run-perf-tests --description "~" > Hmm... It would be also good if we could provide an overall/summarized result: > > I mean something like: > > From 100 FastMalloc results > 69 were better > 31 were worse > > What is your opinion? Yeah, but counting 0.1% improvement and 70% regression equally as "1" is quite misleading.
Ryosuke Niwa
Comment 5 2012-08-24 02:14:06 PDT
Created attachment 160364 [details] New tabular view
Hajime Morrita
Comment 6 2012-08-24 02:22:15 PDT
Coming this late but I'd say that the tabular view is so cool! I hope we can toggle memory / speed view since it's disturbing to investigate memory result freakiness when my main interest is speed. And I guess inverse is also true.
Zoltan Horvath
Comment 7 2012-08-24 02:30:52 PDT
(In reply to comment #4) > Yeah, but counting 0.1% improvement and 70% regression equally as "1" is quite misleading. Absolutely true! (In reply to comment #5) > Created an attachment (id=160364) [details] > New tabular view Looks promising! Shouldn't we use kbytes on the view? Counting with bytes results too big numbers to handle.
Ryosuke Niwa
Comment 8 2012-08-24 03:22:46 PDT
Created attachment 160378 [details] Improved some cosmetics and hides memory results Not sure what kind of UI we need for toggling memory results. Any ideas?
Ryosuke Niwa
Comment 9 2012-08-24 17:03:07 PDT
Created attachment 160524 [details] Added Time/Memory toggle button
Ryosuke Niwa
Comment 10 2012-08-25 02:34:05 PDT
Created attachment 160557 [details] Got rid of scientific notation and added the toggle button for reference run
Ryosuke Niwa
Comment 11 2012-09-14 15:38:49 PDT
Created attachment 164240 [details] Fixes the bug
Ryosuke Niwa
Comment 12 2012-09-14 15:40:17 PDT
Created attachment 164241 [details] Sample output
Eric Seidel (no email)
Comment 13 2012-09-14 15:44:38 PDT
Comment on attachment 164240 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=164240&action=review If you want a JS review, you should talk to arv or ojan or pavel. But I'm happy to rubber stamp this as an improvment over the current UI. > PerformanceTests/resources/jquery.tablesorter.min.js:1 > + I'm surprised we don't have this in the repo already. :) It's such a useful extension...
Ryosuke Niwa
Comment 14 2012-09-14 18:53:20 PDT
Realistically speaking, I'm just looking for a rubber stamp here.
Eric Seidel (no email)
Comment 15 2012-09-14 19:16:08 PDT
Comment on attachment 164240 [details] Fixes the bug That, I can provide!
WebKit Review Bot
Comment 16 2012-09-17 11:16:57 PDT
Comment on attachment 164240 [details] Fixes the bug Clearing flags on attachment: 164240 Committed r128779: <http://trac.webkit.org/changeset/128779>
WebKit Review Bot
Comment 17 2012-09-17 11:17:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.