Chart status should always be computed against prior values
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Apr 2016 16:33:55 +0000 (16:33 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Apr 2016 16:33:55 +0000 (16:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157014

Reviewed by Darin Adler.

Compare the current value against the last baseline or target value that appear before the current value in time
so that the comparison stay the same even when new baseline and target values are reported. Also include the compared
baseline or target value in the label for clarity.

* public/v3/components/chart-status-view.js:
(ChartStatusView.prototype._computeChartStatus):
(ChartStatusView.prototype._computeChartStatus.labelForDiff):
(ChartStatusView.prototype._findLastPointPriorToTime): Extracted from _relativeDifferenceToLaterPointInTimeSeries.
Now finds the last point before the current point's time if there is any, or the last point in baseline / target.
(ChartStatusView.prototype._relativeDifferenceToLaterPointInTimeSeries): Deleted.
* public/v3/models/metric.js:
(Metric.prototype.makeFormatter): Don't use SI units for unit-less metrics.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/components/chart-status-view.js
Websites/perf.webkit.org/public/v3/models/metric.js

index a664ea0..707b970 100644 (file)
@@ -1,3 +1,23 @@
+2016-04-26  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Chart status should always be computed against prior values
+        https://bugs.webkit.org/show_bug.cgi?id=157014
+
+        Reviewed by Darin Adler.
+
+        Compare the current value against the last baseline or target value that appear before the current value in time
+        so that the comparison stay the same even when new baseline and target values are reported. Also include the compared
+        baseline or target value in the label for clarity.
+
+        * public/v3/components/chart-status-view.js:
+        (ChartStatusView.prototype._computeChartStatus):
+        (ChartStatusView.prototype._computeChartStatus.labelForDiff):
+        (ChartStatusView.prototype._findLastPointPriorToTime): Extracted from _relativeDifferenceToLaterPointInTimeSeries.
+        Now finds the last point before the current point's time if there is any, or the last point in baseline / target.
+        (ChartStatusView.prototype._relativeDifferenceToLaterPointInTimeSeries): Deleted.
+        * public/v3/models/metric.js:
+        (Metric.prototype.makeFormatter): Don't use SI units for unit-less metrics.
+
 2016-04-13  Ryosuke Niwa  <rniwa@webkit.org>
 
         REGRESSION(r199444): Perf dashboard always fetches all measurement sets
index a8c351a..0cd4717 100644 (file)
@@ -112,33 +112,38 @@ class ChartStatusView extends ComponentBase {
         if (!currentPoint)
             currentPoint = currentTimeSeriesData[currentTimeSeriesData.length - 1];
 
-        var diffFromBaseline = this._relativeDifferenceToLaterPointInTimeSeries(currentPoint, baselineTimeSeriesData);
-        var diffFromTarget = this._relativeDifferenceToLaterPointInTimeSeries(currentPoint, targetTimeSeriesData);
+        var baselinePoint = this._findLastPointPriorToTime(currentPoint, baselineTimeSeriesData);
+        var diffFromBaseline = baselinePoint !== undefined ? (currentPoint.value - baselinePoint.value) / baselinePoint.value : undefined;
+
+        var targetPoint = this._findLastPointPriorToTime(currentPoint, targetTimeSeriesData);
+        var diffFromTarget = targetPoint !== undefined ? (currentPoint.value - targetPoint.value) / targetPoint.value : undefined;
 
         var label = '';
         var className = '';
 
-        function labelForDiff(diff, name, comparison)
+        function labelForDiff(diff, referencePoint, name, comparison)
         {
-            return Math.abs(diff * 100).toFixed(1) + '% ' + comparison + ' ' + name;
+            var relativeDiff = Math.abs(diff * 100).toFixed(1);
+            var referenceValue = referencePoint ? ` (${formatter(referencePoint.value)})` : '';
+            return `${relativeDiff}% ${comparison} ${name}${referenceValue}`;
         }
 
         var smallerIsBetter = metric.isSmallerBetter();
         if (diffFromBaseline !== undefined && diffFromTarget !== undefined) {
             if (diffFromBaseline > 0 == smallerIsBetter) {
-                label = labelForDiff(diffFromBaseline, 'baseline', 'worse than');
+                label = labelForDiff(diffFromBaseline, baselinePoint, 'baseline', 'worse than');
                 className = 'worse';
             } else if (diffFromTarget < 0 == smallerIsBetter) {
-                label = labelForDiff(diffFromBaseline, 'target', 'better than');
+                label = labelForDiff(diffFromTarget, targetPoint, 'target', 'better than');
                 className = 'better';
             } else
-                label = labelForDiff(diffFromTarget, 'target', 'until');
+                label = labelForDiff(diffFromTarget, targetPoint, 'target', 'until');
         } else if (diffFromBaseline !== undefined) {
             className = diffFromBaseline > 0 == smallerIsBetter ? 'worse' : 'better';
-            label = labelForDiff(diffFromBaseline, 'baseline', className + ' than');
+            label = labelForDiff(diffFromBaseline, baselinePoint, 'baseline', className + ' than');
         } else if (diffFromTarget !== undefined) {
             className = diffFromTarget < 0 == smallerIsBetter ? 'better' : 'worse';
-            label = labelForDiff(diffFromTarget, 'target', className + ' than');
+            label = labelForDiff(diffFromTarget, targetPoint, 'target', className + ' than');
         }
 
         var valueDelta = null;
@@ -157,20 +162,17 @@ class ChartStatusView extends ComponentBase {
         };
     }
 
-    _relativeDifferenceToLaterPointInTimeSeries(currentPoint, timeSeriesData)
+    _findLastPointPriorToTime(currentPoint, timeSeriesData)
     {
         if (!currentPoint || !timeSeriesData || !timeSeriesData.length)
             return undefined;
 
-        // FIXME: We shouldn't be using the first data point to access the time series object.
-        var referencePoint = timeSeriesData[0].series.findPointAfterTime(currentPoint.time);
-        if (!referencePoint)
-            return undefined;
-
-        return (currentPoint.value - referencePoint.value) / referencePoint.value;
+        var i = 0;
+        while (i < timeSeriesData.length - 1 && timeSeriesData[i + 1].time < currentPoint.time)
+            i++;
+        return timeSeriesData[i];
     }
 
-
     static htmlTemplate()
     {
         return `
index a195872..5d3150d 100644 (file)
@@ -68,8 +68,11 @@ class Metric extends LabeledObject {
             isMiliseconds = true;
             unit = 's';
         }
-        var divisor = unit == 'B' ? 1024 : 1000;
 
+        if (!unit)
+            return function (value) { return value.toFixed(2) + ' ' + (unit || ''); }
+
+        var divisor = unit == 'B' ? 1024 : 1000;
         var suffix = ['\u03BC', 'm', '', 'K', 'M', 'G', 'T', 'P', 'E'];
         var threshold = sigFig >= 3 ? divisor : (divisor / 10);
         return function (value) {