<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[210953] trunk/Websites/perf.webkit.org</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/210953">210953</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2017-01-19 22:42:43 -0800 (Thu, 19 Jan 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>measurement-sets API can incorrectly order points with OS version without commit time
https://bugs.webkit.org/show_bug.cgi?id=167227

Reviewed by Chris Dumez.

Ignore revision_order for the purpose of ordering data points in /api/measurement-sets.

These orderings are used in some UI (e.g A/B testing) to order OS build numbers which do not have a timestamp
associated with each &quot;revision&quot;.

The baseline measurements made in our internal dashboard were using these ordering numbers before ordering
results with build time. Because those data points don't have an associated Webkit revisions, all data points
were ordered first by macOS's revision_order, then build time. Because v3 UI completely ignores revision_order
for the purpose of plotting data points, this resulted in some data points being plotted in a wrong order
with some lines going backwards in time.

This patch addresses this discrepancy by stop ordering data points with revision_order in the JSON API.

* public/api/measurement-set.php:
(MeasurementSetFetcher::execute_query): Fixed the bug.
* server-tests/api-measurement-set-tests.js: Added a test.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebsitesperfwebkitorgChangeLog">trunk/Websites/perf.webkit.org/ChangeLog</a></li>
<li><a href="#trunkWebsitesperfwebkitorgpublicapimeasurementsetphp">trunk/Websites/perf.webkit.org/public/api/measurement-set.php</a></li>
<li><a href="#trunkWebsitesperfwebkitorgservertestsapimeasurementsettestsjs">trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkWebsitesperfwebkitorgChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/ChangeLog (210952 => 210953)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/ChangeLog        2017-01-20 05:11:01 UTC (rev 210952)
+++ trunk/Websites/perf.webkit.org/ChangeLog        2017-01-20 06:42:43 UTC (rev 210953)
</span><span class="lines">@@ -1,5 +1,29 @@
</span><span class="cx"> 2017-01-19  Ryosuke Niwa  &lt;rniwa@webkit.org&gt;
</span><span class="cx"> 
</span><ins>+        measurement-sets API can incorrectly order points with OS version without commit time
+        https://bugs.webkit.org/show_bug.cgi?id=167227
+
+        Reviewed by Chris Dumez.
+
+        Ignore revision_order for the purpose of ordering data points in /api/measurement-sets.
+
+        These orderings are used in some UI (e.g A/B testing) to order OS build numbers which do not have a timestamp
+        associated with each &quot;revision&quot;.
+
+        The baseline measurements made in our internal dashboard were using these ordering numbers before ordering
+        results with build time. Because those data points don't have an associated Webkit revisions, all data points
+        were ordered first by macOS's revision_order, then build time. Because v3 UI completely ignores revision_order
+        for the purpose of plotting data points, this resulted in some data points being plotted in a wrong order
+        with some lines going backwards in time.
+
+        This patch addresses this discrepancy by stop ordering data points with revision_order in the JSON API.
+
+        * public/api/measurement-set.php:
+        (MeasurementSetFetcher::execute_query): Fixed the bug.
+        * server-tests/api-measurement-set-tests.js: Added a test.
+
+2017-01-19  Ryosuke Niwa  &lt;rniwa@webkit.org&gt;
+
</ins><span class="cx">         Build fix after r210626. We need to clear Triggerable's static map in each iteration.
</span><span class="cx"> 
</span><span class="cx">         * tools/sync-buildbot.js:
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgpublicapimeasurementsetphp"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/public/api/measurement-set.php (210952 => 210953)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/public/api/measurement-set.php        2017-01-20 05:11:01 UTC (rev 210952)
+++ trunk/Websites/perf.webkit.org/public/api/measurement-set.php        2017-01-20 06:42:43 UTC (rev 210953)
</span><span class="lines">@@ -171,7 +171,7 @@
</span><span class="cx">         return $this-&gt;db-&gt;query('
</span><span class="cx">             SELECT test_runs.*, build_id, build_number, build_builder, build_time,
</span><span class="cx">             array_agg((commit_id, commit_repository, commit_revision, extract(epoch from commit_time at time zone \'utc\') * 1000)) AS revisions,
</span><del>-            extract(epoch from max(commit_time at time zone \'utc\')) * 1000 AS revision_time, max(commit_order) AS revision_order
</del><ins>+            extract(epoch from max(commit_time at time zone \'utc\')) * 1000 AS revision_time
</ins><span class="cx">                 FROM builds
</span><span class="cx">                     LEFT OUTER JOIN build_commits ON commit_build = build_id
</span><span class="cx">                     LEFT OUTER JOIN commits ON build_commit = commit_id, test_runs
</span><span class="lines">@@ -178,7 +178,7 @@
</span><span class="cx">                 WHERE run_build = build_id AND run_config = $1 AND NOT EXISTS (SELECT * FROM build_requests WHERE request_build = build_id)
</span><span class="cx">                 GROUP BY build_id, build_builder, build_number, build_time, build_latest_revision, build_slave,
</span><span class="cx">                     run_id, run_config, run_build, run_iteration_count_cache, run_mean_cache, run_sum_cache, run_square_sum_cache, run_marked_outlier
</span><del>-                ORDER BY revision_time, revision_order, build_time', array($config_id));
</del><ins>+                ORDER BY revision_time, build_time', array($config_id));
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     static function format_map()
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgservertestsapimeasurementsettestsjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js (210952 => 210953)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js        2017-01-20 05:11:01 UTC (rev 210952)
+++ trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js        2017-01-20 06:42:43 UTC (rev 210953)
</span><span class="lines">@@ -372,6 +372,75 @@
</span><span class="cx">         }).catch(done);
</span><span class="cx">     });
</span><span class="cx"> 
</span><ins>+    it(&quot;should order results by build time when commit times are missing&quot;, function (done) {
+        const remote = TestServer.remoteAPI();
+        let repositoryId;
+        addBuilderForReport(reportWithBuildTime[0]).then(() =&gt; {
+            const db = TestServer.database();
+            return Promise.all([
+                db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+                db.insert('commits', {'id': 2, 'repository': 1, 'revision': 'macOS 16A323', 'order': 0}),
+                db.insert('commits', {'id': 3, 'repository': 1, 'revision': 'macOS 16C68', 'order': 1}),
+            ]);
+        }).then(() =&gt; {
+            return remote.postJSON('/api/report/', [{
+                &quot;buildNumber&quot;: &quot;1001&quot;,
+                &quot;buildTime&quot;: '2017-01-19 15:28:01',
+                &quot;revisions&quot;: {
+                    &quot;macOS&quot;: {
+                        &quot;revision&quot;: &quot;macOS 16C68&quot;,
+                    },
+                },
+                &quot;builderName&quot;: &quot;someBuilder&quot;,
+                &quot;builderPassword&quot;: &quot;somePassword&quot;,
+                &quot;platform&quot;: &quot;Sierra&quot;,
+                &quot;tests&quot;: { &quot;Test&quot;: {&quot;metrics&quot;: {&quot;Time&quot;: { &quot;baseline&quot;: [1, 2, 3, 4, 5] } } } },
+            }]);
+        }).then(function () {
+            return remote.postJSON('/api/report/', [{
+                &quot;buildNumber&quot;: &quot;1002&quot;,
+                &quot;buildTime&quot;: '2017-01-19 19:46:37',
+                &quot;revisions&quot;: {
+                    &quot;macOS&quot;: {
+                        &quot;revision&quot;: &quot;macOS 16A323&quot;,
+                    },
+                },
+                &quot;builderName&quot;: &quot;someBuilder&quot;,
+                &quot;builderPassword&quot;: &quot;somePassword&quot;,
+                &quot;platform&quot;: &quot;Sierra&quot;,
+                &quot;tests&quot;: { &quot;Test&quot;: {&quot;metrics&quot;: {&quot;Time&quot;: { &quot;baseline&quot;: [5, 6, 7, 8, 9] } } } },
+            }]);
+        }).then(function () {
+            return queryPlatformAndMetricWithRepository('Sierra', 'Time', 'macOS');
+        }).then(function (result) {
+            return remote.getJSONWithStatus(`/api/measurement-set/?platform=${result.platformId}&amp;metric=${result.metricId}`);
+        }).then(function (response) {
+            const currentRows = response['configurations']['baseline'];
+            assert.equal(currentRows.length, 2);
+            assert.deepEqual(format(response['formatMap'], currentRows[0]), {
+               mean: 3,
+               iterationCount: 5,
+               sum: 15,
+               squareSum: 55,
+               markedOutlier: false,
+               revisions: [[3, 1, 'macOS 16C68', 0]],
+               commitTime: +Date.UTC(2017, 0, 19, 15, 28, 1),
+               buildTime: +Date.UTC(2017, 0, 19, 15, 28, 1),
+               buildNumber: '1001' });
+            assert.deepEqual(format(response['formatMap'], currentRows[1]), {
+                mean: 7,
+                iterationCount: 5,
+                sum: 35,
+                squareSum: 255,
+                markedOutlier: false,
+                revisions: [[2, 1, 'macOS 16A323', 0]],
+                commitTime: +Date.UTC(2017, 0, 19, 19, 46, 37),
+                buildTime: +Date.UTC(2017, 0, 19, 19, 46, 37),
+                buildNumber: '1002' });
+            done();
+        }).catch(done);
+    });
+
</ins><span class="cx">     function buildNumbers(parsedResult, config)
</span><span class="cx">     {
</span><span class="cx">         return parsedResult['configurations'][config].map(function (row) {
</span><span class="lines">@@ -417,7 +486,6 @@
</span><span class="cx">         }).catch(done);
</span><span class="cx">     });
</span><span class="cx"> 
</span><del>-
</del><span class="cx">     it(&quot;should create cached results&quot;, function (done) {
</span><span class="cx">         const remote = TestServer.remoteAPI();
</span><span class="cx">         let cachePrefix;
</span></span></pre>
</div>
</div>

</body>
</html>