Because the number of tests has been increasing and many data are displayed for every test, we need an easy way to show visually the inaccurate results. The criteria I chose is the following: If the standard deviation of the test complexity or the frame rate is more than 10%, the standard deviation and the test name will be displayed in red. If the average frame rate is not in the range = [(desired_frame_rate - 2) .. (desired_frame_rate - 2)], the average frame rate and the test name will be displayed in red.
Created attachment 265524 [details] Patch
Comment on attachment 265524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265524&action=review > PerformanceTests/Animometer/resources/extensions.js:323 > + td.style.color = color || "black"; Can this be done in CSS? > PerformanceTests/Animometer/resources/extensions.js:330 > + td.style.color = color || "black"; Can this be done in CSS? > PerformanceTests/Animometer/resources/extensions.js:372 > + _isAlarmingMeasurement: function(index, data, measurement, options) Maybe "unreliable" or "noisy" instead of "alarming". > PerformanceTests/Animometer/resources/extensions.js:375 > + if (measurement == Strings["JSON_MEASUREMENTS"][3]) > + return data[Strings["JSON_MEASUREMENTS"][3]] >= 10; 3 is a magic number, as is 10. > PerformanceTests/Animometer/resources/extensions.js:378 > + if (index == 1 && measurement == Strings["JSON_MEASUREMENTS"][0]) > + return Math.abs(data[Strings["JSON_MEASUREMENTS"][0]] - options["frame-rate"]) > 2; 0 and 2 are a magic numbers. > PerformanceTests/Animometer/resources/extensions.js:383 > + _isAlarmingTestResults: function(testResults, options) results is plural, so areNoisyTestResults, or isNoisyTestResult > PerformanceTests/Animometer/resources/extensions.js:400 > + td.style.backgroundColor = "yellow"; CSS?. td.classList.add('empty') .empty { backgrouncColor: yellow; } > PerformanceTests/Animometer/resources/extensions.js:434 > + this._showFixedNumber(row, data[measurement], 2, this._isAlarmingMeasurement(index - 2, data, measurement, options) ? "red" : null); "red" should come from CSS. > PerformanceTests/Animometer/runner/resources/animometer.js:49 > + this._resultsTable.showIterations(json[Strings["JSON_RESULTS"][0]], this.options); > sectionsManager.showJSON("json", json[Strings["JSON_RESULTS"][0]][0]); This Strings["JSON_RESULTS"][][] stuff too hard to read.
Created attachment 265618 [details] Patch
Comment on attachment 265524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265524&action=review >> PerformanceTests/Animometer/resources/extensions.js:323 >> + td.style.color = color || "black"; > > Can this be done in CSS? Done. Moved to CSS. >> PerformanceTests/Animometer/resources/extensions.js:330 >> + td.style.color = color || "black"; > > Can this be done in CSS? Done. Text color was moved to CSS. >> PerformanceTests/Animometer/resources/extensions.js:372 >> + _isAlarmingMeasurement: function(index, data, measurement, options) > > Maybe "unreliable" or "noisy" instead of "alarming". Function was renamed to _isNoisyMeasurement(). >> PerformanceTests/Animometer/resources/extensions.js:375 >> + return data[Strings["JSON_MEASUREMENTS"][3]] >= 10; > > 3 is a magic number, as is 10. A constant is used for the noise threshold. We may need to add a UI element to be set by the user. >> PerformanceTests/Animometer/resources/extensions.js:378 >> + return Math.abs(data[Strings["JSON_MEASUREMENTS"][0]] - options["frame-rate"]) > 2; > > 0 and 2 are a magic numbers. A constant is used for the noise threshold. We may need to add a UI element to be set by the user. >> PerformanceTests/Animometer/resources/extensions.js:383 >> + _isAlarmingTestResults: function(testResults, options) > > results is plural, so areNoisyTestResults, or isNoisyTestResult Function was renamed to isNoisyTest()? >> PerformanceTests/Animometer/resources/extensions.js:400 >> + td.style.backgroundColor = "yellow"; > > CSS?. td.classList.add('empty') > > .empty { backgrouncColor: yellow; } Done. Background color was moved to CSS. >> PerformanceTests/Animometer/resources/extensions.js:434 >> + this._showFixedNumber(row, data[measurement], 2, this._isAlarmingMeasurement(index - 2, data, measurement, options) ? "red" : null); > > "red" should come from CSS. Done. Text color was moved to CSS. >> PerformanceTests/Animometer/runner/resources/animometer.js:49 >> sectionsManager.showJSON("json", json[Strings["JSON_RESULTS"][0]][0]); > > This Strings["JSON_RESULTS"][][] stuff too hard to read. Will be addressed in a separate patch.
Comment on attachment 265618 [details] Patch Rejecting attachment 265618 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 265618, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: 824ee64cb6e6faad827b20803d9cae0b8db r192486 = 342967a7ac7e367e3087ec7be1f1857a3ac17553 r192487 = 79e502480923be3865d58e4be67b1a4c49d008f5 r192489 = cd20c088fd743713948cbc21ed53930b5cc1437c r192490 = 7bcfba0371cd3ecbc45cf6c3cba98bd648e069fe r192491 = 4ac83a2d03fe019772cd95bba9292f33a499ce74 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/438578
Created attachment 265630 [details] Patch
Comment on attachment 265630 [details] Patch Clearing flags on attachment: 265630 Committed r192494: <http://trac.webkit.org/changeset/192494>
All reviewed patches have been landed. Closing bug.