|
Description
Jon Lee
2016-02-25 02:09:45 PST
Created attachment 272183 [details]
1. Fix the cursor in the graph analysis when the min complexity is not 0
Created attachment 272184 [details]
2. Consolidate JS files, and move statistics out to a separate JS.
Created attachment 272185 [details]
3. Teach the harness to evaluate the samples and determine the test score.
Created attachment 272186 [details]
4. Allow dropping results JSON.
Created attachment 272187 [details]
5. Fix graph drawing.
Created attachment 272188 [details]
6. Use bootstrapping to get confidence interval in the breakpoint.
Created attachment 272189 [details]
7. Switch to a pseudo-random number generator.
Created attachment 272190 [details]
8. Update test harness reporting.
Created attachment 272191 [details]
9. Add meta charset so that encodings between harness and test match.
Created attachment 272192 [details]
10. Have the bootstrap calculation occur between tests. Clean up harness.
Created attachment 272193 [details]
11. Update the ramp controller.
Created attachment 272194 [details]
12. Tests: update multiply test
Created attachment 272195 [details]
13. Tests: Update distance metric so that fringe tiles get more color.
Created attachment 272196 [details]
14. Tests: update visuals.
Created attachment 272197 [details]
Merged patch sans Tests changes
Created attachment 272198 [details]
Merged patch of only Tests changes
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 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. Committed r197229: <http://trac.webkit.org/changeset/197229> Committed r197230: <http://trac.webkit.org/changeset/197230> follow-up with Dean's comments Committed r197232: <http://trac.webkit.org/changeset/197232> |