Log Message
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.
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (204188 => 204189)
--- 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)
@@ -1,3 +1,31 @@
+2016-08-05 Ryosuke Niwa <[email protected]>
+
+ 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.
+
2016-08-04 Ryosuke Niwa <[email protected]>
MeasurementCluster's addToSeries is slow
Modified: trunk/Websites/perf.webkit.org/public/v3/models/measurement-set.js (204188 => 204189)
--- 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)
@@ -37,16 +37,11 @@
var clusterStart = this._clusterStart;
var clusterSize = this._clusterSize;
- function computeClusterStart(time) {
- var diff = time - clusterStart;
- return clusterStart + Math.floor(diff / clusterSize) * clusterSize;
- }
-
var clusters = [];
- var clusterEnd = computeClusterStart(startTime);
+ var clusterEnd = clusterStart + Math.floor(Math.max(0, startTime - clusterStart) / clusterSize) * clusterSize;
var lastClusterEndTime = this._primaryClusterEndTime;
- var firstClusterEndTime = lastClusterEndTime - clusterStart * this._clusterCount;
+ var firstClusterEndTime = lastClusterEndTime - clusterSize * (this._clusterCount - 1);
do {
clusterEnd += clusterSize;
if (firstClusterEndTime <= clusterEnd && clusterEnd <= this._primaryClusterEndTime)
@@ -189,7 +184,7 @@
Instrumentation.startMeasuringTime('MeasurementSet', 'fetchedTimeSeries');
// FIXME: Properly construct TimeSeries.
- var series = new TimeSeries([]);
+ var series = new TimeSeries();
var idMap = {};
for (var cluster of this._sortedClusters)
cluster.addToSeries(series, configType, includeOutliers, idMap);
Modified: trunk/Websites/perf.webkit.org/unit-tests/analysis-task-tests.js (204188 => 204189)
--- 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)
@@ -151,7 +151,7 @@
requests[0].resolve(sampleAnalysisTask());
var anotherCallCount = 0;
- return promise.then(function () {
+ promise.then(function () {
assert.equal(callCount, 1);
AnalysisTask.fetchAll().then(function () { anotherCallCount++; });
}).then(function () {
@@ -166,7 +166,7 @@
var promise = AnalysisTask.fetchAll();
requests[0].resolve(sampleAnalysisTask());
- return promise.then(function () {
+ promise.then(function () {
assert.equal(AnalysisTask.all().length, 1);
var task = AnalysisTask.all()[0];
assert.equal(task.id(), 1082);
@@ -189,7 +189,7 @@
var promise = AnalysisTask.fetchAll();
requests[0].resolve(sampleAnalysisTask());
- return promise.then(function () {
+ promise.then(function () {
assert.equal(AnalysisTask.all().length, 1);
var task = AnalysisTask.all()[0];
@@ -212,7 +212,7 @@
var promise = AnalysisTask.fetchAll();
requests[0].resolve(sampleAnalysisTask());
- return promise.then(function () {
+ promise.then(function () {
assert.equal(AnalysisTask.all().length, 1);
var task = AnalysisTask.all()[0];
Modified: trunk/Websites/perf.webkit.org/unit-tests/checkconfig.js (204188 => 204189)
--- 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)
@@ -128,7 +128,7 @@
it('should be able to connect to the database', function (done) {
let database = new Database;
- return database.connect().then(function () {
+ database.connect().then(function () {
database.disconnect();
done();
}, function (error) {
Modified: trunk/Websites/perf.webkit.org/unit-tests/measurement-set-tests.js (204188 => 204189)
--- 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)
@@ -34,6 +34,38 @@
});
});
+ 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);
+ });
+ });
+
+ });
+
describe('fetchBetween', function () {
it('should always request the cached primary cluster first', function () {
var set = MeasurementSet.findSet(1, 1, 3000);
@@ -126,7 +158,7 @@
'startTime': 4000,
'endTime': 5000,
'lastModified': 5000,
- 'clusterCount': 3,
+ 'clusterCount': 4,
'status': 'OK'});
var callCount = 0;
@@ -194,7 +226,7 @@
'startTime': 4000,
'endTime': 5000,
'lastModified': 5000,
- 'clusterCount': 3,
+ 'clusterCount': 4,
'status': 'OK'});
var callCount = 0;
@@ -219,6 +251,34 @@
});
});
+ 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);
+ });
+ });
+
it('should invoke the callback when the fetching of the primray cluster fails', function (done) {
var set = MeasurementSet.findSet(1, 1, 3000);
var callCount = 0;
@@ -434,7 +494,7 @@
'startTime': 4000,
'endTime': 5000,
'lastModified': 5000,
- 'clusterCount': 3,
+ 'clusterCount': 4,
'status': 'OK'});
var callCountFor4000To5000 = 0;
@@ -652,7 +712,7 @@
'startTime': 4000,
'endTime': 5000,
'lastModified': 5000,
- 'clusterCount': 2,
+ 'clusterCount': 4,
'status': 'OK'});
waitForMeasurementSet().then(function () {
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
