WebKitPerfMonitor: Sometimes text inside panes overlap
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Mar 2014 06:39:25 +0000 (06:39 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Mar 2014 06:39:25 +0000 (06:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=130956

Reviewed by Gyuyoung Kim.

Revamped the pane UI. Now build info uses table element instead of plane text with BRs. The computed status of
the latest result against baseline/target such as "3% until target" is now shown above the current value. This
reduces the total height of the pane and fits more information per screen capita on the dashboard.

* public/index.html: Updated and added a bunch of CSS rules for the new look.
(.computeStatus): Don't append the build info here. The build info is constructed as a separate table now.
(.createSummaryRowMarkup): Use th instead of td for "Current", "Baseline", and "Target" in the summary table.
(.buildLabelWithLinks): Construct table rows instead of br separated lines of text. This streamlines the look
of the build info shown in a chart pane and a tooltip.
(Chart): Made .status a table.
(Chart.populate): Prepend status.text, which contains text such as "3% until target", into the summary rows
right above "Current" value, and populate .status with buildLabelWithLinks manually instead of status.text
now that status.text no longer contains it.
(Chart..showTooltipWithResults):  Wrap buildLabelWithLinks with a table element.

* public/js/helper-classes.js:
(TestBuild.formattedRevisions): Don't include repository names in labels since repository names are now added
by buildLabelWithLinks inside th elements. Also place spaces around '-' between two different OS X versions.
e.g. "OS X 10.8 - OS X 10.9" instead of "OS X 10.8-OS X 10.9".
(PerfTestRuns): Use "/s" for "runs/s" and "B" for "bytes" to make text shorter in .status and .summaryTable.
(PerfTestRuns..computeScalingFactorIfNeeded): Avoid placing a space between 'M' and a unit starting with a
capital letter; e.g. "MB" instead of "M B".

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/index.html
Websites/perf.webkit.org/public/js/helper-classes.js

index af3c65c..6d4b2bb 100644 (file)
@@ -1,5 +1,35 @@
 2014-03-30  Ryosuke Niwa  <rniwa@webkit.org>
 
+        WebKitPerfMonitor: Sometimes text inside panes overlap
+        https://bugs.webkit.org/show_bug.cgi?id=130956
+
+        Reviewed by Gyuyoung Kim.
+
+        Revamped the pane UI. Now build info uses table element instead of plane text with BRs. The computed status of
+        the latest result against baseline/target such as "3% until target" is now shown above the current value. This
+        reduces the total height of the pane and fits more information per screen capita on the dashboard.
+
+        * public/index.html: Updated and added a bunch of CSS rules for the new look.
+        (.computeStatus): Don't append the build info here. The build info is constructed as a separate table now.
+        (.createSummaryRowMarkup): Use th instead of td for "Current", "Baseline", and "Target" in the summary table.
+        (.buildLabelWithLinks): Construct table rows instead of br separated lines of text. This streamlines the look
+        of the build info shown in a chart pane and a tooltip.
+        (Chart): Made .status a table.
+        (Chart.populate): Prepend status.text, which contains text such as "3% until target", into the summary rows
+        right above "Current" value, and populate .status with buildLabelWithLinks manually instead of status.text
+        now that status.text no longer contains it.
+        (Chart..showTooltipWithResults):  Wrap buildLabelWithLinks with a table element.
+
+        * public/js/helper-classes.js:
+        (TestBuild.formattedRevisions): Don't include repository names in labels since repository names are now added
+        by buildLabelWithLinks inside th elements. Also place spaces around '-' between two different OS X versions.
+        e.g. "OS X 10.8 - OS X 10.9" instead of "OS X 10.8-OS X 10.9".
+        (PerfTestRuns): Use "/s" for "runs/s" and "B" for "bytes" to make text shorter in .status and .summaryTable.
+        (PerfTestRuns..computeScalingFactorIfNeeded): Avoid placing a space between 'M' and a unit starting with a
+        capital letter; e.g. "MB" instead of "M B".
+
+2014-03-30  Ryosuke Niwa  <rniwa@webkit.org>
+
         WebKitPerfMonitor: Header and number-of-days slider takes up too much space
         https://bugs.webkit.org/show_bug.cgi?id=130957
 
index ac6e570..2b4e549 100644 (file)
@@ -40,10 +40,6 @@ td, th {
     font-weight: normal;
 }
 
-#dashboard td {
-    
-}
-
 .chart {
     position: relative;
     border: solid 1px #ccc;
@@ -73,8 +69,7 @@ td, th {
 }
 
 #dashboard header {
-    padding: 10px;
-    width: 200px;
+    padding: 0px 10px;
 }
 
 .chart h2, .chart h3 {
@@ -82,7 +77,6 @@ td, th {
     padding: 0;
     font-size: 1em;
     font-weight: normal;
-    width: 200px;
     word-break: break-all;
 }
 
@@ -98,6 +92,10 @@ td, th {
     display: none;
 }
 
+#dashboard .chart .status {
+    margin: 0px 10px;
+}
+
 .chart .status {
     font-size: small;
     color: #666;
@@ -115,6 +113,10 @@ td, th {
     height: 15px;
 }
 
+#dashboard .chart {
+    width: 410px;
+}
+
 #dashboard .plot {
     width: 400px;
     height: 100px;
@@ -123,7 +125,7 @@ td, th {
 }
 
 #charts .plot {
-    height: 300px;
+    height: 320px;
     margin-left: 250px;
 }
 
@@ -137,17 +139,41 @@ td, th {
     height: 70px;
 }
 
-.summaryTable {
+.chart .summaryTable {
     font-size: small;
     color: #666;
     border: 0;
 }
 
-#dashboard .summaryTable {
+.chart .meta {
+    position: relative;
+}
+
+.chart .meta table,
+.chart .meta td,
+.tooltip table,
+.tooltip td {
+    margin: 0;
+    padding: 0;
+}
+
+.chart .meta th,
+.tooltip th {
+    margin: 0;
+    padding: 0 0.2em 0 0;
+    text-align: right;
+    font-weight: normal;
+}
+
+.chart .meta td:not(:first-child):before,
+.tooltip td:not(:first-child):before {
+    content: ": ";
+}
+
+#dashboard .chart .summaryTable {
     position: absolute;
     right: 10px;
-    top: 10px;
-    width: 180px;
+    top: 0px;
 }
 
 #dashboard .arrow {
@@ -218,7 +244,7 @@ td, th {
     color: #eee;
     font-size: small;
     line-height: 130%;
-    width: 30ex;
+    white-space: nowrap;
 }
 
 .tooltip:after {
@@ -320,10 +346,6 @@ td, th {
         } else if (relativeDifferenceWithTarget)
             statusText = Math.abs(relativeDifferenceWithTarget * 100).toFixed(2) + '% until target';
 
-        if (statusText)
-            statusText += '<br>';
-        statusText += buildLabelWithLinks(lastResult.build());
-
         return {class: status, text: statusText};
     }
 
@@ -344,7 +366,7 @@ td, th {
     }
 
     function createSummaryRowMarkup(name, runs) {
-        return '<tr><td>' + name + '</td><td>' + runs.lastResult().label() + '</td></tr>';
+        return '<tr><th>' + name + '</th><td>' + runs.lastResult().label() + '</td></tr>';
     }
 
     function buildLabelWithLinks(build, previousBuild) {
@@ -353,11 +375,16 @@ td, th {
         }
 
         var formattedRevisions = build.formattedRevisions(previousBuild);
-        var markup = ['Committed: ' + build.formattedTime(),
-            'Build: ' + linkifyIfNotNull(build.buildNumber(), build.buildUrl()) + ' (' + build.formattedBuildTime() + ')'];
+        var buildInfo = {
+            'Commit': build.formattedTime(),
+            'Build': linkifyIfNotNull(build.buildNumber(), build.buildUrl()) + '(' + build.formattedBuildTime() + ')'
+        };
         for (var repositoryName in formattedRevisions)
-            markup.push(linkifyIfNotNull(formattedRevisions[repositoryName].label, formattedRevisions[repositoryName].url));
-        return markup.join('<br>');
+            buildInfo[repositoryName] = linkifyIfNotNull(formattedRevisions[repositoryName].label, formattedRevisions[repositoryName].url);
+        var markup = '';
+        for (var key in buildInfo)
+            markup += '<tr><th>' + key + '</th><td>' + buildInfo[key] + '</td>';
+        return '<tr>' + markup + '</tr>';
     }
 
     function Chart(container, isDashboard, platform, metric, bugTrackers, onClose) {
@@ -365,9 +392,8 @@ td, th {
         if (metric.test.url)
             linkifiedFullName = '<a href="' + metric.test.url + '">' + linkifiedFullName + '</a>';
         var section = $('<section class="chart"><div class="pane"><header><h2>' + linkifiedFullName + '</h2>'
-            + '<h3 class="platform">' + platform.name + '</h3>'
-            + '<span class="status"></span></header>'
-            + '<table class="summaryTable"><tbody></tbody></table>'
+            + '<h3 class="platform">' + platform.name + '</h3></header>'
+            + '<div class="meta"><table class="status"><tbody></tbody></table><table class="summaryTable"><tbody></tbody></table></div>'
             + '<div class="overviewPlot"></div></div>'
             + '<div class="plot"></div>'
             + '<div class="unit"></div>'
@@ -440,8 +466,10 @@ td, th {
             summaryRows = createSummaryRowMarkup('Current', results) + summaryRows;
 
             var status = computeStatus(results.smallerIsBetter(), results.lastResult(), baseline, target);
+            if (status.text)
+                summaryRows = '<tr><td colspan="2">' + status.text + '</td></tr>' + summaryRows;
             section.addClass(status.class);
-            section.find('.status').html(status.text);
+            section.find('.status tbody').html(buildLabelWithLinks(results.lastResult().build()));
             section.find('.summaryTable tbody').html(summaryRows);
             section.find('.unit').html(results.unit());
             if (results.smallerIsBetter()) {
@@ -571,8 +599,8 @@ td, th {
                 }
                 newBugUrls = 'File:' + newBugUrls;
             }
-            tooltip.show(x, y, result.label(resultToCompare) + '<br>'
-                + buildLabelWithLinks(result.build(), resultToCompare ? resultToCompare.build() : null) + '<br>'
+            tooltip.show(x, y, result.label(resultToCompare) + '<table>'
+                + buildLabelWithLinks(result.build(), resultToCompare ? resultToCompare.build() : null) + '</table>'
                 + newBugUrls);
         }
 
index 8789ff4..0626bda 100755 (executable)
@@ -68,26 +68,28 @@ function TestBuild(repositories, builders, platform, rawRun) {
     this.revision = function(repositoryName) { return revisions[repositoryName][0]; }
     this.formattedRevisions = function (previousBuild) {
         var result = {};
-        for (var repositoryName in revisions) {
+        for (var repositoryName in repositories) {
+            if (!revisions[repositoryName])
+                continue;
             var previousRevision = previousBuild ? previousBuild.revision(repositoryName) : undefined;
             var currentRevision = this.revision(repositoryName);
             if (previousRevision === currentRevision)
                 previousRevision = undefined;
 
             var revisionPrefix = '';
-            if (currentRevision.length < 10) { // SVN-like revision.
+            var revisionDelimitor = '-';
+            if (parseInt(currentRevision) == currentRevision) { // e.g. r12345.
                 revisionPrefix = 'r';
                 if (previousRevision)
                     previousRevision = (parseInt(previousRevision) + 1);
-            }
+            } else if (currentRevision.indexOf(' ') >= 0) // e.g. 10.9 13C64.
+                revisionDelimitor = ' - ';
 
-            var labelForThisRepository = revisionCount ? repositoryName : '';
-            if (previousRevision) {
-                if (labelForThisRepository)
-                    labelForThisRepository += ' ';
-                labelForThisRepository += revisionPrefix + previousRevision + '-' + revisionPrefix + currentRevision;
-            } else
-                labelForThisRepository += ' @ ' + revisionPrefix + currentRevision;
+            var labelForThisRepository;
+            if (previousRevision)
+                labelForThisRepository = revisionPrefix + previousRevision + revisionDelimitor + revisionPrefix + currentRevision;
+            else
+                labelForThisRepository = '@ ' + revisionPrefix + currentRevision;
 
             var url;
             var repository = repositories[repositoryName];
@@ -124,14 +126,14 @@ function PerfTestRuns(metric, platform) {
     var baselines = {};
     var unit = {'Combined': '', // Assume smaller is better for now.
         'FrameRate': 'fps',
-        'Runs': 'runs/s',
+        'Runs': '/s',
         'Time': 'ms',
-        'Malloc': 'bytes',
-        'JSHeap': 'bytes',
-        'Allocations': 'bytes',
-        'EndAllocations': 'bytes',
-        'MaxAllocations': 'bytes',
-        'MeanAllocations': 'bytes'}[metric.name];
+        'Malloc': 'B',
+        'JSHeap': 'B',
+        'Allocations': 'B',
+        'EndAllocations': 'B',
+        'MaxAllocations': 'B',
+        'MeanAllocations': 'B'}[metric.name];
 
     // We can't do this in PerfTestResult because all results for each metric need to share the same unit and the same scaling factor.
     function computeScalingFactorIfNeeded() {
@@ -141,10 +143,11 @@ function PerfTestRuns(metric, platform) {
             return;
 
         var mean = results[0].unscaledMean(); // FIXME: We should look at all values.
-        var kilo = unit == 'bytes' ? 1024 : 1000;
+        var kilo = unit == 'B' ? 1024 : 1000;
         if (mean > 2 * kilo * kilo && unit != 'ms') {
             cachedScalingFactor = 1 / kilo / kilo;
-            cachedUnit = 'M ' + unit;
+            var unitFirstChar = unit.charAt(0);
+            cachedUnit = 'M' + (unitFirstChar.toUpperCase() == unitFirstChar ? '' : ' ') + unit;
         } else if (mean > 2 * kilo) {
             cachedScalingFactor = 1 / kilo;
             cachedUnit = unit == 'ms' ? 's' : ('K ' + unit);