<!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>[204189] 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/204189">204189</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2016-08-05 13:21:35 -0700 (Fri, 05 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Perf dashboard sometimes tries to fetch a non-existent measurement-set JSON
https://bugs.webkit.org/show_bug.cgi?id=160577
Rubber-stamped by Chris Dumez.
The bug was caused by findClusters computing the first cluster's endTime incorrectly. Namely, we were
multiplying the number of clusters by clusterStart instead of clusterSize with an off-by-one error.
* public/v3/models/measurement-set.js:
(MeasurementSet.prototype.findClusters): Folded computeClusterStart into where clusterEnd is computed
for clarity. Also fixed a bug that we were not computing the first cluster to fetch correctly when
the fetched time range started before clusterStart (i.e. when startTime - clusterStart is negative).
Finally, fixed the main bug by multiplying the number of clusters by clusterSize instead of
clusterStart to compute the end time of the very first cluster in this measurement set. Because what
we're computing here is the end time of the first cluster, not the start time, we also need to subtract
one from the number of clusters. e.g. if there was exactly one cluster, then firstClusterEndTime is
identically equal to lastClusterEndTime.
(MeasurementSet.prototype.fetchedTimeSeries): Removed the unused argument to TimeSeries's constructor.
* unit-tests/analysis-task-tests.js: Fixed the tests for the latest version of Mocha which complains if
we returned a promise in unit tests when "done" function is used.
* unit-tests/checkconfig.js: Ditto.
* unit-tests/measurement-set-tests.js: Added a test case for findClusters and a test to make sure
fetchBetween doesn't try to fetch a cluster before the first cluster in the set. Also fixed other test
cases which were relying on the bug this patch fixed.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebsitesperfwebkitorgChangeLog">trunk/Websites/perf.webkit.org/ChangeLog</a></li>
<li><a href="#trunkWebsitesperfwebkitorgpublicv3modelsmeasurementsetjs">trunk/Websites/perf.webkit.org/public/v3/models/measurement-set.js</a></li>
<li><a href="#trunkWebsitesperfwebkitorgunittestsanalysistasktestsjs">trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js</a></li>
<li><a href="#trunkWebsitesperfwebkitorgunittestscheckconfigjs">trunk/Websites/perf.webkit.org/unit-tests/checkconfig.js</a></li>
<li><a href="#trunkWebsitesperfwebkitorgunittestsmeasurementsettestsjs">trunk/Websites/perf.webkit.org/unit-tests/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 (204188 => 204189)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/ChangeLog 2016-08-05 20:19:51 UTC (rev 204188)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2016-08-05 20:21:35 UTC (rev 204189)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2016-08-05 Ryosuke Niwa <rniwa@webkit.org>
+
+ Perf dashboard sometimes tries to fetch a non-existent measurement-set JSON
+ https://bugs.webkit.org/show_bug.cgi?id=160577
+
+ Rubber-stamped by Chris Dumez.
+
+ The bug was caused by findClusters computing the first cluster's endTime incorrectly. Namely, we were
+ multiplying the number of clusters by clusterStart instead of clusterSize with an off-by-one error.
+
+ * public/v3/models/measurement-set.js:
+ (MeasurementSet.prototype.findClusters): Folded computeClusterStart into where clusterEnd is computed
+ for clarity. Also fixed a bug that we were not computing the first cluster to fetch correctly when
+ the fetched time range started before clusterStart (i.e. when startTime - clusterStart is negative).
+ Finally, fixed the main bug by multiplying the number of clusters by clusterSize instead of
+ clusterStart to compute the end time of the very first cluster in this measurement set. Because what
+ we're computing here is the end time of the first cluster, not the start time, we also need to subtract
+ one from the number of clusters. e.g. if there was exactly one cluster, then firstClusterEndTime is
+ identically equal to lastClusterEndTime.
+ (MeasurementSet.prototype.fetchedTimeSeries): Removed the unused argument to TimeSeries's constructor.
+
+ * unit-tests/analysis-task-tests.js: Fixed the tests for the latest version of Mocha which complains if
+ we returned a promise in unit tests when "done" function is used.
+ * unit-tests/checkconfig.js: Ditto.
+ * unit-tests/measurement-set-tests.js: Added a test case for findClusters and a test to make sure
+ fetchBetween doesn't try to fetch a cluster before the first cluster in the set. Also fixed other test
+ cases which were relying on the bug this patch fixed.
+
</ins><span class="cx"> 2016-08-04 Ryosuke Niwa <rniwa@webkit.org>
</span><span class="cx">
</span><span class="cx"> MeasurementCluster's addToSeries is slow
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgpublicv3modelsmeasurementsetjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/public/v3/models/measurement-set.js (204188 => 204189)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/public/v3/models/measurement-set.js 2016-08-05 20:19:51 UTC (rev 204188)
+++ trunk/Websites/perf.webkit.org/public/v3/models/measurement-set.js 2016-08-05 20:21:35 UTC (rev 204189)
</span><span class="lines">@@ -37,16 +37,11 @@
</span><span class="cx"> var clusterStart = this._clusterStart;
</span><span class="cx"> var clusterSize = this._clusterSize;
</span><span class="cx">
</span><del>- function computeClusterStart(time) {
- var diff = time - clusterStart;
- return clusterStart + Math.floor(diff / clusterSize) * clusterSize;
- }
-
</del><span class="cx"> var clusters = [];
</span><del>- var clusterEnd = computeClusterStart(startTime);
</del><ins>+ var clusterEnd = clusterStart + Math.floor(Math.max(0, startTime - clusterStart) / clusterSize) * clusterSize;
</ins><span class="cx">
</span><span class="cx"> var lastClusterEndTime = this._primaryClusterEndTime;
</span><del>- var firstClusterEndTime = lastClusterEndTime - clusterStart * this._clusterCount;
</del><ins>+ var firstClusterEndTime = lastClusterEndTime - clusterSize * (this._clusterCount - 1);
</ins><span class="cx"> do {
</span><span class="cx"> clusterEnd += clusterSize;
</span><span class="cx"> if (firstClusterEndTime <= clusterEnd && clusterEnd <= this._primaryClusterEndTime)
</span><span class="lines">@@ -189,7 +184,7 @@
</span><span class="cx"> Instrumentation.startMeasuringTime('MeasurementSet', 'fetchedTimeSeries');
</span><span class="cx">
</span><span class="cx"> // FIXME: Properly construct TimeSeries.
</span><del>- var series = new TimeSeries([]);
</del><ins>+ var series = new TimeSeries();
</ins><span class="cx"> var idMap = {};
</span><span class="cx"> for (var cluster of this._sortedClusters)
</span><span class="cx"> cluster.addToSeries(series, configType, includeOutliers, idMap);
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgunittestsanalysistasktestsjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js (204188 => 204189)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js 2016-08-05 20:19:51 UTC (rev 204188)
+++ trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js 2016-08-05 20:21:35 UTC (rev 204189)
</span><span class="lines">@@ -151,7 +151,7 @@
</span><span class="cx"> requests[0].resolve(sampleAnalysisTask());
</span><span class="cx">
</span><span class="cx"> var anotherCallCount = 0;
</span><del>- return promise.then(function () {
</del><ins>+ promise.then(function () {
</ins><span class="cx"> assert.equal(callCount, 1);
</span><span class="cx"> AnalysisTask.fetchAll().then(function () { anotherCallCount++; });
</span><span class="cx"> }).then(function () {
</span><span class="lines">@@ -166,7 +166,7 @@
</span><span class="cx"> var promise = AnalysisTask.fetchAll();
</span><span class="cx"> requests[0].resolve(sampleAnalysisTask());
</span><span class="cx">
</span><del>- return promise.then(function () {
</del><ins>+ promise.then(function () {
</ins><span class="cx"> assert.equal(AnalysisTask.all().length, 1);
</span><span class="cx"> var task = AnalysisTask.all()[0];
</span><span class="cx"> assert.equal(task.id(), 1082);
</span><span class="lines">@@ -189,7 +189,7 @@
</span><span class="cx"> var promise = AnalysisTask.fetchAll();
</span><span class="cx"> requests[0].resolve(sampleAnalysisTask());
</span><span class="cx">
</span><del>- return promise.then(function () {
</del><ins>+ promise.then(function () {
</ins><span class="cx"> assert.equal(AnalysisTask.all().length, 1);
</span><span class="cx"> var task = AnalysisTask.all()[0];
</span><span class="cx">
</span><span class="lines">@@ -212,7 +212,7 @@
</span><span class="cx"> var promise = AnalysisTask.fetchAll();
</span><span class="cx"> requests[0].resolve(sampleAnalysisTask());
</span><span class="cx">
</span><del>- return promise.then(function () {
</del><ins>+ promise.then(function () {
</ins><span class="cx"> assert.equal(AnalysisTask.all().length, 1);
</span><span class="cx"> var task = AnalysisTask.all()[0];
</span><span class="cx">
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgunittestscheckconfigjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/unit-tests/checkconfig.js (204188 => 204189)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/unit-tests/checkconfig.js 2016-08-05 20:19:51 UTC (rev 204188)
+++ trunk/Websites/perf.webkit.org/unit-tests/checkconfig.js 2016-08-05 20:21:35 UTC (rev 204189)
</span><span class="lines">@@ -128,7 +128,7 @@
</span><span class="cx">
</span><span class="cx"> it('should be able to connect to the database', function (done) {
</span><span class="cx"> let database = new Database;
</span><del>- return database.connect().then(function () {
</del><ins>+ database.connect().then(function () {
</ins><span class="cx"> database.disconnect();
</span><span class="cx"> done();
</span><span class="cx"> }, function (error) {
</span></span></pre></div>
<a id="trunkWebsitesperfwebkitorgunittestsmeasurementsettestsjs"></a>
<div class="modfile"><h4>Modified: trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js (204188 => 204189)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js 2016-08-05 20:19:51 UTC (rev 204188)
+++ trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js 2016-08-05 20:21:35 UTC (rev 204189)
</span><span class="lines">@@ -34,6 +34,38 @@
</span><span class="cx"> });
</span><span class="cx"> });
</span><span class="cx">
</span><ins>+ describe('findClusters', function () {
+
+ it('should return clusters that exist', function (done) {
+ var set = MeasurementSet.findSet(1, 1, 1467852503940);
+ var callCount = 0;
+ var promise = set.fetchBetween(1465084800000, 1470268800000, function () {
+ callCount++;
+ });
+ assert.equal(requests.length, 1);
+ assert.equal(requests[0].url, '../data/measurement-set-1-1.json');
+
+ requests[0].resolve({
+ 'clusterStart': 946684800000,
+ 'clusterSize': 5184000000,
+ 'formatMap': [],
+ 'configurations': {current: []},
+ 'startTime': 1465084800000,
+ 'endTime': 1470268800000,
+ 'lastModified': 1467852503940,
+ 'clusterCount': 5,
+ 'status': 'OK'});
+
+ promise.then(function () {
+ assert.deepEqual(set.findClusters(0, Date.now()), [1449532800000, 1454716800000, 1459900800000, 1465084800000, 1470268800000]);
+ done();
+ }).catch(function (error) {
+ done(error);
+ });
+ });
+
+ });
+
</ins><span class="cx"> describe('fetchBetween', function () {
</span><span class="cx"> it('should always request the cached primary cluster first', function () {
</span><span class="cx"> var set = MeasurementSet.findSet(1, 1, 3000);
</span><span class="lines">@@ -126,7 +158,7 @@
</span><span class="cx"> 'startTime': 4000,
</span><span class="cx"> 'endTime': 5000,
</span><span class="cx"> 'lastModified': 5000,
</span><del>- 'clusterCount': 3,
</del><ins>+ 'clusterCount': 4,
</ins><span class="cx"> 'status': 'OK'});
</span><span class="cx">
</span><span class="cx"> var callCount = 0;
</span><span class="lines">@@ -194,7 +226,7 @@
</span><span class="cx"> 'startTime': 4000,
</span><span class="cx"> 'endTime': 5000,
</span><span class="cx"> 'lastModified': 5000,
</span><del>- 'clusterCount': 3,
</del><ins>+ 'clusterCount': 4,
</ins><span class="cx"> 'status': 'OK'});
</span><span class="cx">
</span><span class="cx"> var callCount = 0;
</span><span class="lines">@@ -219,6 +251,34 @@
</span><span class="cx"> });
</span><span class="cx"> });
</span><span class="cx">
</span><ins>+ it('should not request a cluster before the very first cluster', function (done) {
+ var set = MeasurementSet.findSet(1, 1, 5000);
+ set.fetchBetween(0, 3000, function () {
+ assert.notReached();
+ });
+ assert.equal(requests.length, 1);
+ assert.equal(requests[0].url, '../data/measurement-set-1-1.json');
+
+ requests[0].resolve({
+ 'clusterStart': 2000,
+ 'clusterSize': 1000,
+ 'formatMap': [],
+ 'configurations': {current: []},
+ 'startTime': 2000,
+ 'endTime': 3000,
+ 'lastModified': 5000,
+ 'clusterCount': 1,
+ 'status': 'OK'});
+
+ var callCount = 0;
+ waitForMeasurementSet().then(function () {
+ assert.equal(requests.length, 1);
+ done();
+ }).catch(function (error) {
+ done(error);
+ });
+ });
+
</ins><span class="cx"> it('should invoke the callback when the fetching of the primray cluster fails', function (done) {
</span><span class="cx"> var set = MeasurementSet.findSet(1, 1, 3000);
</span><span class="cx"> var callCount = 0;
</span><span class="lines">@@ -434,7 +494,7 @@
</span><span class="cx"> 'startTime': 4000,
</span><span class="cx"> 'endTime': 5000,
</span><span class="cx"> 'lastModified': 5000,
</span><del>- 'clusterCount': 3,
</del><ins>+ 'clusterCount': 4,
</ins><span class="cx"> 'status': 'OK'});
</span><span class="cx">
</span><span class="cx"> var callCountFor4000To5000 = 0;
</span><span class="lines">@@ -652,7 +712,7 @@
</span><span class="cx"> 'startTime': 4000,
</span><span class="cx"> 'endTime': 5000,
</span><span class="cx"> 'lastModified': 5000,
</span><del>- 'clusterCount': 2,
</del><ins>+ 'clusterCount': 4,
</ins><span class="cx"> 'status': 'OK'});
</span><span class="cx">
</span><span class="cx"> waitForMeasurementSet().then(function () {
</span></span></pre>
</div>
</div>
</body>
</html>