Bug 154673 - Update animation benchmark and tests
Summary: Update animation benchmark and tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jon Lee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-25 02:09 PST by Jon Lee
Modified: 2016-02-26 20:44 PST (History)
6 users (show)

See Also:


Attachments
1. Fix the cursor in the graph analysis when the min complexity is not 0 (3.19 KB, patch)
2016-02-25 02:11 PST, Jon Lee
no flags Details | Formatted Diff | Diff
2. Consolidate JS files, and move statistics out to a separate JS. (40.80 KB, patch)
2016-02-25 02:11 PST, Jon Lee
no flags Details | Formatted Diff | Diff
3. Teach the harness to evaluate the samples and determine the test score. (77.45 KB, patch)
2016-02-25 02:11 PST, Jon Lee
no flags Details | Formatted Diff | Diff
4. Allow dropping results JSON. (5.24 KB, patch)
2016-02-25 02:12 PST, Jon Lee
no flags Details | Formatted Diff | Diff
5. Fix graph drawing. (4.87 KB, patch)
2016-02-25 02:12 PST, Jon Lee
no flags Details | Formatted Diff | Diff
6. Use bootstrapping to get confidence interval in the breakpoint. (26.43 KB, patch)
2016-02-25 02:12 PST, Jon Lee
no flags Details | Formatted Diff | Diff
7. Switch to a pseudo-random number generator. (10.67 KB, patch)
2016-02-25 02:13 PST, Jon Lee
no flags Details | Formatted Diff | Diff
8. Update test harness reporting. (8.49 KB, patch)
2016-02-25 02:13 PST, Jon Lee
no flags Details | Formatted Diff | Diff
9. Add meta charset so that encodings between harness and test match. (14.52 KB, patch)
2016-02-25 02:13 PST, Jon Lee
no flags Details | Formatted Diff | Diff
10. Have the bootstrap calculation occur between tests. Clean up harness. (14.31 KB, patch)
2016-02-25 02:14 PST, Jon Lee
no flags Details | Formatted Diff | Diff
11. Update the ramp controller. (24.33 KB, patch)
2016-02-25 02:17 PST, Jon Lee
no flags Details | Formatted Diff | Diff
12. Tests: update multiply test (3.65 KB, patch)
2016-02-25 02:18 PST, Jon Lee
no flags Details | Formatted Diff | Diff
13. Tests: Update distance metric so that fringe tiles get more color. (1.73 KB, patch)
2016-02-25 02:18 PST, Jon Lee
no flags Details | Formatted Diff | Diff
14. Tests: update visuals. (6.56 KB, patch)
2016-02-25 02:18 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Merged patch sans Tests changes (182.10 KB, patch)
2016-02-25 02:22 PST, Jon Lee
dino: review+
Details | Formatted Diff | Diff
Merged patch of only Tests changes (9.97 KB, patch)
2016-02-25 02:25 PST, Jon Lee
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2016-02-25 02:09:45 PST
Refine the ramp controller, and use bootstrapping to calculate confidence intervals.
Comment 1 Jon Lee 2016-02-25 02:11:01 PST
Created attachment 272183 [details]
1. Fix the cursor in the graph analysis when the min  complexity is not 0
Comment 2 Jon Lee 2016-02-25 02:11:24 PST
Created attachment 272184 [details]
2. Consolidate JS files, and move statistics out to a  separate JS.
Comment 3 Jon Lee 2016-02-25 02:11:57 PST
Created attachment 272185 [details]
3. Teach the harness to evaluate the samples and determine  the test score.
Comment 4 Jon Lee 2016-02-25 02:12:20 PST
Created attachment 272186 [details]
4. Allow dropping results JSON.
Comment 5 Jon Lee 2016-02-25 02:12:35 PST
Created attachment 272187 [details]
5. Fix graph drawing.
Comment 6 Jon Lee 2016-02-25 02:12:51 PST
Created attachment 272188 [details]
6. Use bootstrapping to get confidence interval in the  breakpoint.
Comment 7 Jon Lee 2016-02-25 02:13:14 PST
Created attachment 272189 [details]
7. Switch to a pseudo-random number generator.
Comment 8 Jon Lee 2016-02-25 02:13:32 PST
Created attachment 272190 [details]
8. Update test harness reporting.
Comment 9 Jon Lee 2016-02-25 02:13:49 PST
Created attachment 272191 [details]
9. Add meta charset so that encodings between harness and  test match.
Comment 10 Jon Lee 2016-02-25 02:14:11 PST
Created attachment 272192 [details]
10. Have the bootstrap calculation occur between tests.  Clean up harness.
Comment 11 Jon Lee 2016-02-25 02:17:42 PST
Created attachment 272193 [details]
11. Update the ramp controller.
Comment 12 Jon Lee 2016-02-25 02:18:01 PST
Created attachment 272194 [details]
12. Tests: update multiply test
Comment 13 Jon Lee 2016-02-25 02:18:16 PST
Created attachment 272195 [details]
13. Tests: Update distance metric so that fringe tiles get  more color.
Comment 14 Jon Lee 2016-02-25 02:18:39 PST
Created attachment 272196 [details]
14. Tests: update visuals.
Comment 15 Jon Lee 2016-02-25 02:22:49 PST
Created attachment 272197 [details]
Merged patch sans Tests changes
Comment 16 Jon Lee 2016-02-25 02:25:34 PST
Created attachment 272198 [details]
Merged patch of only Tests changes
Comment 17 Dean Jackson 2016-02-26 13:38:39 PST
Comment on attachment 272197 [details]
Merged patch sans Tests changes

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

Giving this a provisional r+ assuming Said will do an informal review. I'm not going to pretend I understood everything :)

> PerformanceTests/Animometer/resources/debug-runner/animometer.js:31
> +        button.textContent = Strings.text.graph + "...";

Typographic nerds would prefer "…" :)

> PerformanceTests/Animometer/resources/debug-runner/animometer.js:500
> +        if (dashboard.options["adjustment"] == "ramp") {
> +            Headers.details[3].disabled = true;
> +        } else {

Nit: Single line conditional

> PerformanceTests/Animometer/resources/debug-runner/graph.js:9
> +        element.testResult = testResult;
> +        element.testData = testData;

More of a general comment, but I wonder if we should prefix these custom attrs/properties with _

> PerformanceTests/Animometer/resources/debug-runner/graph.js:595
> +            document.getElementById("complexity-regression-aggregate-raw").textContent = complexityRegression.complexity.toFixed(2) + ", ±" + complexityRegression.stdev.toFixed(2) + "ms";

I guess this is just a review tool bug - supposed to be the +/- sign?

> PerformanceTests/Animometer/resources/debug-runner/graph.js:603
> +                "â",

Same here, but not sure what it is supposed to be.

> PerformanceTests/Animometer/resources/extensions.js:467
> +        for (var i = 0; i < this._size; ++i) {
> +            out += this._values[i];
> +            if (i < this._size - 1)
> +                out += ", ";
> +        }

Can you use this._values.join(", ") here?

$ jsc
>>> [1, 2, 3, 4, 5].join(", ")
1, 2, 3, 4, 5
Comment 18 Jon Lee 2016-02-26 14:05:28 PST
Comment on attachment 272197 [details]
Merged patch sans Tests changes

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

>> PerformanceTests/Animometer/resources/debug-runner/animometer.js:31
>> +        button.textContent = Strings.text.graph + "...";
> 
> Typographic nerds would prefer "…" :)

Done.

>> PerformanceTests/Animometer/resources/debug-runner/animometer.js:500
>> +        } else {
> 
> Nit: Single line conditional

Done.

>> PerformanceTests/Animometer/resources/debug-runner/graph.js:9
>> +        element.testData = testData;
> 
> More of a general comment, but I wonder if we should prefix these custom attrs/properties with _

We could. Done.

>> PerformanceTests/Animometer/resources/debug-runner/graph.js:595
>> +            document.getElementById("complexity-regression-aggregate-raw").textContent = complexityRegression.complexity.toFixed(2) + ", ±" + complexityRegression.stdev.toFixed(2) + "ms";
> 
> I guess this is just a review tool bug - supposed to be the +/- sign?

Yes, it is.

>> PerformanceTests/Animometer/resources/debug-runner/graph.js:603
>> +                "â",
> 
> Same here, but not sure what it is supposed to be.

en dash.

>> PerformanceTests/Animometer/resources/extensions.js:467
>> +        }
> 
> Can you use this._values.join(", ") here?
> 
> $ jsc

I didn't touch this code; I was just moving it, but yes we could do that.
Comment 19 Jon Lee 2016-02-26 20:32:14 PST
Committed r197229: <http://trac.webkit.org/changeset/197229>
Comment 20 Jon Lee 2016-02-26 20:33:30 PST
Committed r197230: <http://trac.webkit.org/changeset/197230>
Comment 21 Jon Lee 2016-02-26 20:44:18 PST
follow-up with Dean's comments
Committed r197232: <http://trac.webkit.org/changeset/197232>