Make v3 UI analysis task page is hard to understand
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jan 2016 21:57:32 +0000 (21:57 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jan 2016 21:57:32 +0000 (21:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152917

Reviewed by Antti Koivisto.

Add a dark gray border around the selected block in the analysis results viewer instead of using darker
shades since that looks as if they were bigger regression/progression.

Explicitly show "Failed" as the label instead of omitting with "-" when all build requests in an A/B
testing group fails.

* public/v3/components/analysis-results-viewer.js:
(AnalysisResultsViewer.cssTemplate): Tweaked the style to underline text in the hovered blocks and the
selected blocks and show a dark gray border around the selected blocks.
(AnalysisResultsViewer.TestGroupStackingBlock):
(AnalysisResultsViewer.TestGroupStackingBlock.prototype.createStackingCell): Use this._title for title.
(AnalysisResultsViewer.TestGroupStackingBlock.prototype._computeTestGroupStatus):
(AnalysisResultsViewer.TestGroupStackingBlock.prototype._valuesForRootSet): Deleted.

* public/v3/components/results-table.js:
(ResultsTable.prototype.render):
(ResultsTable.prototype._createRevisionListCells): Extracted from ResultsTable.prototype.render.
(ResultsTable.cssTemplate): Tweaked the style.
(ResultsTableRow):
(ResultsTableRow.prototype.constructor): Added _labelForWholeRow to store the label for the entire row.
This is used to show the comparison result of two root sets (e.g. A vs B).
(ResultsTableRow.prototype.setLabelForWholeRow): Added.
(ResultsTableRow.prototype.labelForWholeRow): Added.
(ResultsTableRow.prototype.resultContent): Extracted from buildHeading. Creates a hyperlinked bar graph
used for each A/B testing result.
(ResultsTableRow.prototype.buildHeading): Deleted since we need to set colspan on the second table cell
when we're creating a row with _labelForWholeRow.

* public/v3/components/test-group-results-table.js:
(TestGroupResultsTable.prototype.buildRowGroups): Added rows to show relative differences and statistical
significance between root sets (e.g. A vs B).

* public/v3/models/build-request.js:
(BuildRequest.prototype.hasCompleted): Added.

* public/v3/models/test-group.js:
(TestGroup.prototype.compareTestResults): Extracted from AnalysisResultsViewer.TestGroupStackingBlock's
_computeTestGroupStatus and generalized to be reused in TestGroupResultsTable.
(TestGroup.prototype._valuesForRootSet): Moved from AnalysisResultsViewer.TestGroupStackingBlock.

* public/v3/pages/analysis-task-page.js:
(AnalysisTaskPage.cssTemplate): Tweaked the style.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194788 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
Websites/perf.webkit.org/public/v3/components/results-table.js
Websites/perf.webkit.org/public/v3/components/test-group-results-table.js
Websites/perf.webkit.org/public/v3/models/build-request.js
Websites/perf.webkit.org/public/v3/models/test-group.js
Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js

index d24f1bb..9620318 100644 (file)
@@ -1,3 +1,53 @@
+2016-01-08  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Make v3 UI analysis task page is hard to understand
+        https://bugs.webkit.org/show_bug.cgi?id=152917
+
+        Reviewed by Antti Koivisto.
+
+        Add a dark gray border around the selected block in the analysis results viewer instead of using darker
+        shades since that looks as if they were bigger regression/progression.
+
+        Explicitly show "Failed" as the label instead of omitting with "-" when all build requests in an A/B
+        testing group fails.
+
+        * public/v3/components/analysis-results-viewer.js:
+        (AnalysisResultsViewer.cssTemplate): Tweaked the style to underline text in the hovered blocks and the
+        selected blocks and show a dark gray border around the selected blocks.
+        (AnalysisResultsViewer.TestGroupStackingBlock):
+        (AnalysisResultsViewer.TestGroupStackingBlock.prototype.createStackingCell): Use this._title for title.
+        (AnalysisResultsViewer.TestGroupStackingBlock.prototype._computeTestGroupStatus):
+        (AnalysisResultsViewer.TestGroupStackingBlock.prototype._valuesForRootSet): Deleted.
+
+        * public/v3/components/results-table.js:
+        (ResultsTable.prototype.render):
+        (ResultsTable.prototype._createRevisionListCells): Extracted from ResultsTable.prototype.render.
+        (ResultsTable.cssTemplate): Tweaked the style.
+        (ResultsTableRow):
+        (ResultsTableRow.prototype.constructor): Added _labelForWholeRow to store the label for the entire row.
+        This is used to show the comparison result of two root sets (e.g. A vs B).
+        (ResultsTableRow.prototype.setLabelForWholeRow): Added.
+        (ResultsTableRow.prototype.labelForWholeRow): Added.
+        (ResultsTableRow.prototype.resultContent): Extracted from buildHeading. Creates a hyperlinked bar graph
+        used for each A/B testing result.
+        (ResultsTableRow.prototype.buildHeading): Deleted since we need to set colspan on the second table cell
+        when we're creating a row with _labelForWholeRow.
+
+        * public/v3/components/test-group-results-table.js:
+        (TestGroupResultsTable.prototype.buildRowGroups): Added rows to show relative differences and statistical
+        significance between root sets (e.g. A vs B).
+
+        * public/v3/models/build-request.js:
+        (BuildRequest.prototype.hasCompleted): Added.
+
+        * public/v3/models/test-group.js:
+        (TestGroup.prototype.compareTestResults): Extracted from AnalysisResultsViewer.TestGroupStackingBlock's
+        _computeTestGroupStatus and generalized to be reused in TestGroupResultsTable.
+        (TestGroup.prototype._valuesForRootSet): Moved from AnalysisResultsViewer.TestGroupStackingBlock.
+
+        * public/v3/pages/analysis-task-page.js:
+        (AnalysisTaskPage.cssTemplate): Tweaked the style.
+
 2016-01-07  Ryosuke Niwa  <rniwa@webkit.org>
 
         Perf dashboard should automatically add aggregators
index f84483f..6a0fd7c 100644 (file)
@@ -239,6 +239,7 @@ class AnalysisResultsViewer extends ResultsTable {
     {
         return ResultsTable.cssTemplate() + `
             .analysis-view .stacking-block {
+                position: relative;
                 border: solid 1px #fff;
                 cursor: pointer;
             }
@@ -249,62 +250,43 @@ class AnalysisResultsViewer extends ResultsTable {
                 color: inherit;
                 font-size: 0.8rem;
                 padding: 0 0.1rem;
+                max-width: 3rem;
             }
 
             .analysis-view .stacking-block:not(.failed) {
-                color: white;
+                color: black;
                 opacity: 1;
             }
 
-            .analysis-view .stacking-block.selected {
+            .analysis-view .stacking-block.selected,
+            .analysis-view .stacking-block:hover {
+                text-decoration: underline;
+            }
 
+            .analysis-view .stacking-block.selected:before {
+                content: '';
+                position: absolute;
+                left: 0px;
+                top: 0px;
+                width: calc(100% - 2px);
+                height: calc(100% - 2px);
+                border: solid 1px #333;
             }
 
             .analysis-view .stacking-block.failed {
-                background: rgba(128, 51, 128, 0.4);
-            }
-            .analysis-view .stacking-block.failed:hover {
-                background: rgba(128, 51, 128, 0.6);
+                background: rgba(128, 51, 128, 0.5);
             }
-            .analysis-view .stacking-block.failed.selected {
-                background: rgba(128, 51, 128, 1);
-                color: white;
-            }
-
             .analysis-view .stacking-block.unchanged {
-                background: rgba(128, 128, 128, 0.3);
-                color: black;
-            }
-            .analysis-view .stacking-block.unchanged:hover {
-                background: rgba(128, 128, 128, 0.6);
+                background: rgba(128, 128, 128, 0.5);
             }
-            .analysis-view .stacking-block.unchanged.selected {
-                background: rgba(128, 128, 128, 1);
-                color: white;
+            .analysis-view .stacking-block.incomplete {
+                background: rgba(204, 204, 51, 0.5);
             }
-
             .analysis-view .stacking-block.worse {
-                background: rgba(255, 102, 102, 0.4);
-                color: black;
-            }
-            .analysis-view .stacking-block.worse:hover {
-                background: rgba(255, 102, 102, 0.6);
+                background: rgba(255, 102, 102, 0.5);
             }
-            .analysis-view .stacking-block.worse.selected {
-                background: rgba(255, 102, 102, 1);
-                color: white;
-            }
-
             .analysis-view .stacking-block.better {
-                background: rgba(102, 102, 255, 0.4);
-                color: black;
-            }
-            .analysis-view .stacking-block.better:hover {
-                background: rgba(102, 102, 255, 0.6);
-            }
-            .analysis-view .stacking-block.better.selected {
-                background: rgba(102, 102, 255, 1);
-                color: white;
+                background: rgba(102, 102, 255, 0.5);
             }
         `;
     }
@@ -332,7 +314,8 @@ AnalysisResultsViewer.TestGroupStackingBlock = class {
         this._smallerIsBetter = smallerIsBetter;
         this._rootSetIndexRowIndexMap = [];
         this._className = className;
-        this._label = '-';
+        this._label = null;
+        this._title = null;
         this._status = null;
         this._callback = callback;
     }
@@ -349,13 +332,12 @@ AnalysisResultsViewer.TestGroupStackingBlock = class {
     {
         this._computeTestGroupStatus();
 
-        var title = this._testGroup.label();
         return ComponentBase.createElement('td', {
             rowspan: this.endRowIndex() - this.startRowIndex() + 1,
-            title: title,
+            title: this._title,
             class: 'stacking-block ' + this._className + ' ' + this._status,
             onclick: this._callback,
-        }, ComponentBase.createLink(this._label, title, this._callback));
+        }, ComponentBase.createLink(this._label, this._title, this._callback));
     }
 
     isComplete() { return this._rootSetIndexRowIndexMap.length >= 2; }
@@ -375,32 +357,12 @@ AnalysisResultsViewer.TestGroupStackingBlock = class {
 
         console.assert(this._rootSetIndexRowIndexMap.length <= 2); // FIXME: Support having more root sets.
 
-        var beforeValues = this._valuesForRootSet(this._rootSetIndexRowIndexMap[0].rootSet);
-        var afterValues = this._valuesForRootSet(this._rootSetIndexRowIndexMap[1].rootSet);
-
-        var status = 'failed';
-        var beforeMean = Statistics.sum(beforeValues) / beforeValues.length;
-        var afterMean = Statistics.sum(afterValues) / afterValues.length;
-        if (beforeValues.length && afterValues.length) {
-            var diff = afterMean - beforeMean;
-            this._label = (diff / beforeMean * 100).toFixed(2) + '%';
-            status = 'unchanged';
-            if (Statistics.testWelchsT(beforeValues, afterValues))
-                status = diff < 0 == this._smallerIsBetter ? 'better' : 'worse';
-        }
-
-        this._status = status;
-    }
+        var result = this._testGroup.compareTestResults(
+            this._rootSetIndexRowIndexMap[0].rootSet, this._rootSetIndexRowIndexMap[1].rootSet);
 
-    _valuesForRootSet(rootSet)
-    {
-        var requests = this._testGroup.requestsForRootSet(rootSet);
-        var values = [];
-        for (var request of requests) {
-            if (request.result())
-                values.push(request.result().value);
-        }
-        return values;
+        this._label = result.label;
+        this._title = result.fullLabel;
+        this._status = result.status;
     }
 }
 
index 46f8c6e..7ab183d 100644 (file)
@@ -22,50 +22,24 @@ class ResultsTable extends ComponentBase {
 
         var barGraphGroup = new BarGraphGroup(this._valueFormatter);
         var element = ComponentBase.createElement;
-        var link = ComponentBase.createLink;
+        var self = this;
         var tableBodies = rowGroups.map(function (group) {
             var groupHeading = group.heading;
             var revisionSupressionCount = {};
 
             return element('tbody', group.rows.map(function (row, rowIndex) {
-                var cells = row.buildHeading(barGraphGroup);
+                var cells = [];
 
-                if (groupHeading && !rowIndex)
-                    cells.unshift(element('th', {rowspan: group.rows.length}, groupHeading));
+                if (groupHeading !== undefined && !rowIndex)
+                    cells.push(element('th', {rowspan: group.rows.length}, groupHeading));
+                cells.push(element('td', row.heading()));
 
-                var rootSet = row.rootSet();
-                repositoryList.forEach(function (repository) {
-                    var commit = rootSet ? rootSet.commitForRepository(repository) : null;
-
-                    if (revisionSupressionCount[repository.id()]) {
-                        revisionSupressionCount[repository.id()]--;
-                        return;
-                    }
-
-                    var succeedingRowIndex = rowIndex + 1;
-                    while (succeedingRowIndex < group.rows.length) {
-                        var succeedingRootSet = group.rows[succeedingRowIndex].rootSet();
-                        if (succeedingRootSet && commit != succeedingRootSet.commitForRepository(repository))
-                            break;
-                        succeedingRowIndex++;
-                    }
-                    var rowSpan = succeedingRowIndex - rowIndex;
-                    var attributes = {class: 'revision'};
-                    if (rowSpan > 1) {
-                        revisionSupressionCount[repository.id()] = rowSpan - 1;
-                        attributes['rowspan'] = rowSpan;                       
-                    }
-                    if (rowIndex + rowSpan >= group.rows.length)
-                        attributes['class'] += ' lastRevision';
-
-                    var content = 'Missing';
-                    if (commit) {
-                        var url = commit.url();
-                        content = url ? link(commit.label(), url) : commit.label();
-                    }
-
-                    cells.push(element('td', attributes, content));
-                });
+                if (row.labelForWholeRow())
+                    cells.push(element('td', {class: 'whole-row-label', colspan: repositoryList.length + 1}, row.labelForWholeRow()));
+                else {
+                    cells.push(element('td', row.resultContent(barGraphGroup)));
+                    cells.push(self._createRevisionListCells(repositoryList, revisionSupressionCount, group, row.rootSet(), rowIndex));
+                }
 
                 return element('tr', [cells, row.additionalColumns()]);
             }));
@@ -89,6 +63,46 @@ class ResultsTable extends ComponentBase {
         Instrumentation.endMeasuringTime('ResultsTable', 'render');
     }
 
+    _createRevisionListCells(repositoryList, revisionSupressionCount, testGroup, rootSet, rowIndex)
+    {
+        var element = ComponentBase.createElement;
+        var link = ComponentBase.createLink;
+        var cells = [];
+        for (var repository of repositoryList) {
+            var commit = rootSet ? rootSet.commitForRepository(repository) : null;
+
+            if (revisionSupressionCount[repository.id()]) {
+                revisionSupressionCount[repository.id()]--;
+                continue;
+            }
+
+            var succeedingRowIndex = rowIndex + 1;
+            while (succeedingRowIndex < testGroup.rows.length) {
+                var succeedingRootSet = testGroup.rows[succeedingRowIndex].rootSet();
+                if (succeedingRootSet && commit != succeedingRootSet.commitForRepository(repository))
+                    break;
+                succeedingRowIndex++;
+            }
+            var rowSpan = succeedingRowIndex - rowIndex;
+            var attributes = {class: 'revision'};
+            if (rowSpan > 1) {
+                revisionSupressionCount[repository.id()] = rowSpan - 1;
+                attributes['rowspan'] = rowSpan;                       
+            }
+            if (rowIndex + rowSpan >= testGroup.rows.length)
+                attributes['class'] += ' lastRevision';
+
+            var content = 'Missing';
+            if (commit) {
+                var url = commit.url();
+                content = url ? link(commit.label(), url) : commit.label();
+            }
+
+            cells.push(element('td', attributes, content));
+        }
+        return cells;
+    }
+
     heading() { throw 'NotImplemented'; }
     additionalHeading() { return []; }
     buildRowGroups() { throw 'NotImplemented'; }
@@ -152,7 +166,11 @@ class ResultsTable extends ComponentBase {
                 height: 1.4rem;
                 text-align: center;
             }
-            
+
+            .results-table td.whole-row-label {
+                text-align: left;
+            }
+
             .results-table thead {
                 color: #c93;
             }
@@ -228,10 +246,11 @@ class ResultsTableRow {
         this._label = '-';
         this._rootSet = rootSet;
         this._additionalColumns = [];
+        this._labelForWholeRow = null;
     }
 
+    heading() { return this._heading; }
     rootSet() { return this._rootSet; }
-
     setResult(result) { this._result = result; }
     setLink(link, label)
     {
@@ -239,18 +258,15 @@ class ResultsTableRow {
         this._label = label;
     }
 
+    setLabelForWholeRow(label) { this._labelForWholeRow = label; }
+    labelForWholeRow() { return this._labelForWholeRow; }
+
     additionalColumns() { return this._additionalColumns; }
     setAdditionalColumns(additionalColumns) { this._additionalColumns = additionalColumns; }
 
-    buildHeading(barGraphGroup)
+    resultContent(barGraphGroup)
     {
-        var element = ComponentBase.createElement;
-        var link = ComponentBase.createLink;
-
         var resultContent = this._result ? barGraphGroup.addBar(this._result.value, this._result.interval) : this._label;
-        return [
-            element('th', this._heading),
-            element('td', this._link ? link(resultContent, this._label, this._link) : resultContent),
-        ];
+        return this._link ? ComponentBase.createLink(resultContent, this._label, this._link) : resultContent;
     }
 }
index f1db25b..38bbb45 100644 (file)
@@ -29,12 +29,14 @@ class TestGroupResultsTable extends ResultsTable {
         if (!testGroup)
             return [];
 
-        return this._testGroup.requestedRootSets().map(function (rootSet, setIndex) {
+        var rootSets = this._testGroup.requestedRootSets();
+        var groups = rootSets.map(function (rootSet, setIndex) {
             var rows = [new ResultsTableRow('Mean', rootSet)];
             var results = [];
 
             for (var request of testGroup.requestsForRootSet(rootSet)) {
                 var result = request.result();
+                // Call result.rootSet() for each result since the set of revisions used in testing maybe different from requested ones.
                 var row = new ResultsTableRow(1 + +request.order(), result ? result.rootSet() : null);
                 rows.push(row);
                 if (result) {
@@ -50,7 +52,27 @@ class TestGroupResultsTable extends ResultsTable {
                 rows[0].setResult(aggregatedResult);
 
             return {heading: String.fromCharCode('A'.charCodeAt(0) + setIndex), rows:rows};
-        })
+        });
+
+        var comparisonRows = [];
+        for (var i = 0; i < rootSets.length; i++) {
+            for (var j = i + 1; j < rootSets.length; j++) {
+                var startConfig = String.fromCharCode('A'.charCodeAt(0) + i);
+                var endConfig = String.fromCharCode('A'.charCodeAt(0) + j);
+
+                var result = this._testGroup.compareTestResults(rootSets[i], rootSets[j]);
+                if (result.status == 'incomplete' || result.status == 'failed')
+                    continue;
+
+                var row = new ResultsTableRow(`${startConfig} to ${endConfig}`, null);
+                row.setLabelForWholeRow(result.fullLabel);
+                comparisonRows.push(row);
+            }
+        }
+
+        groups.push({heading: '', rows: comparisonRows});
+
+        return groups;
     }
 }
 
index 63b01fc..93973c1 100644 (file)
@@ -20,6 +20,7 @@ class BuildRequest extends DataModelObject {
     order() { return this._order; }
     rootSet() { return this._rootSet; }
 
+    hasCompleted() { return this._status == 'failed' || this._status == 'completed'; }
     statusLabel()
     {
         switch (this._status) {
index 5ecab8f..47b1750 100644 (file)
@@ -59,6 +59,50 @@ class TestGroup extends LabeledObject {
         this._allRootSets = null;
     }
 
+    hasCompleted()
+    {
+        return this._buildRequests.every(function (request) { return request.hasCompleted(); });
+    }
+
+    compareTestResults(rootSetA, rootSetB)
+    {
+        var beforeValues = this._valuesForRootSet(rootSetA);
+        var afterValues = this._valuesForRootSet(rootSetB);
+        var beforeMean = Statistics.sum(beforeValues) / beforeValues.length;
+        var afterMean = Statistics.sum(afterValues) / afterValues.length;
+
+        var result = {changeType: null, status: 'failed', label: 'Failed', fullLabel: 'Failed', isStatisticallySignificant: false};
+        if (beforeValues.length && afterValues.length) {
+            var diff = afterMean - beforeMean;
+            result.changeType = diff < 0 == this._smallerIsBetter ? 'better' : 'worse';
+            result.label = Math.abs(diff / beforeMean * 100).toFixed(2) + '% ' + result.changeType;
+            result.isStatisticallySignificant = Statistics.testWelchsT(beforeValues, afterValues);
+            result.status = result.isStatisticallySignificant ? result.changeType : 'unchanged';
+        }
+
+        if (!this.hasCompleted()) {
+            result.status = 'incomplete';
+            result.label = 'Running';
+            result.fullLabel = 'Running';
+        } else if (result.changeType) {
+            var significance = result.isStatisticallySignificant ? 'significant' : 'insignificant';
+            result.fullLabel = `${result.label} (statistically ${significance})`;
+        }
+
+        return result;
+    }
+
+    _valuesForRootSet(rootSet)
+    {
+        var requests = this.requestsForRootSet(rootSet);
+        var values = [];
+        for (var request of requests) {
+            if (request.result())
+                values.push(request.result().value);
+        }
+        return values;
+    }
+
     static fetchByTask(taskId)
     {
         return this.cachedFetch('../api/test-groups', {task: taskId}).then(function (data) {
index 41e026f..8639170 100644 (file)
@@ -204,11 +204,8 @@ class AnalysisTaskPage extends PageWithHeading {
     {
         return `
             .analysis-tasl-page-container {
-                text-align: center;
             }
             .analysis-tasl-page {
-                display: inline-block;
-                text-align: left;
             }
 
             .analysis-task-name {
@@ -245,34 +242,34 @@ class AnalysisTaskPage extends PageWithHeading {
                 margin: 1rem;
             }
 
+            .test-configuration h3 {
+                font-size: 1rem;
+                font-weight: inherit;
+                color: inherit;
+                margin: 0 1rem;
+                padding: 0;
+            }
+
             .test-group-view {
-                display: flex;
-                flex-direction: row;
-                align-items: stretch;
-                align-content: stretch;
+                display: table;
                 margin: 0 1rem;
             }
 
             .test-group-details {
-                display: flex;
-                flex-grow: 1;
+                display: table-cell;
                 margin-bottom: 1rem;
             }
 
-            .test-configuration h3 {
-                font-size: 1rem;
-                font-weight: inherit;
-                color: inherit;
-                margin: 0 1rem;
-                padding: 0;
+            .test-group-list {
+                display: table-cell;
             }
 
             .test-group-list:not(:empty) {
                 margin: 0;
                 padding: 0.2rem 0;
                 list-style: none;
-                display: inline-block;
                 border-right: solid 1px #ccc;
+                white-space: nowrap;
             }
 
             .test-group-list li {