Bug 141438 - New perf dashboard should have the ability to overlay moving average with an envelope
Summary: New perf dashboard should have the ability to overlay moving average with an ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Perf Dashboard (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 141237 141443
  Show dependency treegraph
 
Reported: 2015-02-10 11:03 PST by Ryosuke Niwa
Modified: 2016-02-16 13:53 PST (History)
4 users (show)

See Also:


Attachments
Patch (34.90 KB, patch)
2015-02-10 13:22 PST, Ryosuke Niwa
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>