Bug 94668 - Perf test results is incomprehensive
Summary: Perf test results is incomprehensive
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 77037
  Show dependency treegraph
 
Reported: 2012-08-21 21:07 PDT by Ryosuke Niwa
Modified: 2012-09-17 11:17 PDT (History)
11 users (show)

See Also:


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 Details | Formatted Diff | Diff
Sample output (139.54 KB, text/html)
2012-09-14 15:40 PDT, Ryosuke Niwa
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 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.
Comment 2 Ryosuke Niwa 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.
Comment 3 Zoltan Horvath 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?
Comment 4 Ryosuke Niwa 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.
Comment 5 Ryosuke Niwa 2012-08-24 02:14:06 PDT
Created attachment 160364 [details]
New tabular view
Comment 6 Hajime Morrita 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.
Comment 7 Zoltan Horvath 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.
Comment 8 Ryosuke Niwa 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?
Comment 9 Ryosuke Niwa 2012-08-24 17:03:07 PDT
Created attachment 160524 [details]
Added Time/Memory toggle button
Comment 10 Ryosuke Niwa 2012-08-25 02:34:05 PDT
Created attachment 160557 [details]
Got rid of scientific notation and added the toggle button for reference run
Comment 11 Ryosuke Niwa 2012-09-14 15:38:49 PDT
Created attachment 164240 [details]
Fixes the bug
Comment 12 Ryosuke Niwa 2012-09-14 15:40:17 PDT
Created attachment 164241 [details]
Sample output
Comment 13 Eric Seidel (no email) 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...
Comment 14 Ryosuke Niwa 2012-09-14 18:53:20 PDT
Realistically speaking, I'm just looking for a rubber stamp here.
Comment 15 Eric Seidel (no email) 2012-09-14 19:16:08 PDT
Comment on attachment 164240 [details]
Fixes the bug

That, I can provide!
Comment 16 WebKit Review Bot 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>
Comment 17 WebKit Review Bot 2012-09-17 11:17:01 PDT
All reviewed patches have been landed.  Closing bug.