Bug 141438

Summary: New perf dashboard should have the ability to overlay moving average with an envelope
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Perf DashboardAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, kling, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 141237, 141443    
Attachments:
Description Flags
Patch kling: review+

Description Ryosuke Niwa 2015-02-10 11:03:48 PST
In order to hide outliers and detect new regressions automatically, we need a way to try out different kinds of moving averages and enveloping strategies.
Add the ability to show that visually on our graphs.
Comment 1 Ryosuke Niwa 2015-02-10 13:22:13 PST
Created attachment 246340 [details]
Patch
Comment 2 Andreas Kling 2015-02-10 13:25:02 PST
Comment on attachment 246340 [details]
Patch

rs=me
Comment 3 Chris Dumez 2015-02-10 13:26:43 PST
Comment on attachment 246340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=246340&action=review

> Websites/perf.webkit.org/public/v2/app.js:441
> +        var strategies = Statistics.EnvelopingStrategies.map(this._cloneStrategy.bind(this));

Isn't it confusing to reuse the same variable name as before?

> Websites/perf.webkit.org/public/v2/app.js:447
> +        var parametreList = (strategy.parameterList || []).map(function (param) { return Ember.Object.create(param); });

parametreList -> parameterList
Comment 4 Chris Dumez 2015-02-10 13:29:34 PST
Comment on attachment 246340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=246340&action=review

> Websites/perf.webkit.org/public/v2/app.js:466
> +        for (var i = 0; i < chosenStrategy.parameters; i++)

chosenStrategy.parameters.length ?
Comment 5 Chris Dumez 2015-02-10 13:34:52 PST
Comment on attachment 246340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=246340&action=review

> Websites/perf.webkit.org/public/v2/js/statistics.js:112
> +                var sum = 0;

redundant? already inside the loop.

> Websites/perf.webkit.org/public/v2/js/statistics.js:113
> +                var i = 0;

redundant? already inside the loop.
Comment 6 Ryosuke Niwa 2015-02-10 13:38:28 PST
Thanks for the reviews. Will land addresssing those comments.
Comment 7 Ryosuke Niwa 2015-02-10 13:39:41 PST
Committed r179878: <http://trac.webkit.org/changeset/179878>