Log Message
commit time returned by '/api/measurement-set' should match the one returned by '/api/commits'. https://bugs.webkit.org/show_bug.cgi?id=191457
Reviewed by Dean Jackson and Ryosuke Niwa. Commit time returned by '/api/measurement-set' sometimes is calculated by 'epoch from ..'. This function will return a floating number with 5 or 6 decimal digits due to double precision limitations. However, some commits may be reported with 6 decimal decimal. So the commit time for those commits will be rounded to 5 decimal digits. In order to avoid front end assertion failure in CommitLog, Database::to_js_time need to match this behavior. * public/include/db.php: Change the behavior to match that of postgres. Added logic to avoid losing precision in php. * server-tests/api-measurement-set-tests.js: Added unit tests for this bug. (queryPlatformAndMetric): Fix a bug that arguments are not used at all.
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (238163 => 238164)
--- trunk/Websites/perf.webkit.org/ChangeLog 2018-11-14 04:58:17 UTC (rev 238163)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2018-11-14 05:31:45 UTC (rev 238164)
@@ -1,3 +1,21 @@
+2018-11-08 Dewei Zhu <dewei_...@apple.com>
+
+ commit time returned by '/api/measurement-set' should match the one returned by '/api/commits'.
+ https://bugs.webkit.org/show_bug.cgi?id=191457
+
+ Reviewed by Dean Jackson and Ryosuke Niwa.
+
+ Commit time returned by '/api/measurement-set' sometimes is calculated by 'epoch from ..'.
+ This function may return a floating number with 5 or 6 decimal digits due to double precision limitations.
+ However, some commits may be reported with 6 decimal decimal.
+ So the commit time for those commits will sometime be rounded to 5 decimal digits.
+ In order to avoid front end assertion failure in CommitLog, Database::to_js_time need to round to 5 digits.
+
+ * public/include/db.php: Change the behavior to match that of postgres.
+ Added logic to avoid losing precision in php.
+ * server-tests/api-measurement-set-tests.js: Added unit tests for this bug.
+ (queryPlatformAndMetric): Fix a bug that arguments are not used at all.
+
2018-11-07 Dewei Zhu <dewei_...@apple.com>
"/api/report" does not check commit time correctly.
Modified: trunk/Websites/perf.webkit.org/public/include/db.php (238163 => 238164)
--- trunk/Websites/perf.webkit.org/public/include/db.php 2018-11-14 04:58:17 UTC (rev 238163)
+++ trunk/Websites/perf.webkit.org/public/include/db.php 2018-11-14 05:31:45 UTC (rev 238164)
@@ -100,11 +100,14 @@
}
static function to_js_time($time_str) {
- $timestamp_in_s = strtotime($time_str);
+ $timestamp_in_ms = strtotime($time_str) * 1000;
$dot_index = strrpos($time_str, '.');
- if ($dot_index !== FALSE)
- $timestamp_in_s += floatval(substr($time_str, $dot_index));
- return intval($timestamp_in_s * 1000);
+ if ($dot_index !== FALSE) {
+ // Keep 5 decimal digits as postgres timestamp may only have 5 decimal digits.
+ // Multiply by 1000 ahead to avoid losing precision. 1538041792.670479 will become 1538041792.6705 on php.
+ $timestamp_in_ms += round(floatval(substr($time_str, $dot_index)), 5) * 1000;
+ }
+ return intval($timestamp_in_ms);
}
static function escape_for_like($string) {
Modified: trunk/Websites/perf.webkit.org/public/include/report-processor.php (238163 => 238164)
--- trunk/Websites/perf.webkit.org/public/include/report-processor.php 2018-11-14 04:58:17 UTC (rev 238163)
+++ trunk/Websites/perf.webkit.org/public/include/report-processor.php 2018-11-14 05:31:45 UTC (rev 238164)
@@ -172,7 +172,7 @@
if (!$commit_row)
$this->rollback_with_error('FailedToRecordCommit', $commit_data);
- if ($commit_data['time'] && abs(strtotime($commit_row['commit_time']) - strtotime($commit_data['time'])) > 1.0)
+ if ($commit_data['time'] && abs(Database::to_js_time($commit_row['commit_time']) - Database::to_js_time($commit_data['time'])) > 1000.0)
$this->rollback_with_error('MismatchingCommitTime', array('existing' => $commit_row, 'new' => $commit_data));
if (!$this->db->select_or_insert_row('build_commits', null,
Modified: trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js (238163 => 238164)
--- trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js 2018-11-14 04:58:17 UTC (rev 238163)
+++ trunk/Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js 2018-11-14 05:31:45 UTC (rev 238164)
@@ -16,8 +16,8 @@
{
const db = TestServer.database();
return Promise.all([
- db.selectFirstRow('platforms', {name: 'Mountain Lion'}),
- db.selectFirstRow('test_metrics', {name: 'Time'}),
+ db.selectFirstRow('platforms', {name: platformName}),
+ db.selectFirstRow('test_metrics', {name: metricName}),
]).then(function (result) {
return {platformId: result[0]['id'], metricId: result[1]['id']};
});
@@ -545,4 +545,91 @@
1, 1362046323388, '123', 1, 1, 'current']]);
});
+ const reportWithCommitsNeedsRoundCommitTimeAndBuildRequest = {
+ "buildNumber": "123",
+ "buildTime": "2013-02-28T10:12:03.388304",
+ "builderName": "someBuilder",
+ "builderPassword": "somePassword",
+ "platform": "Mountain Lion",
+ "buildRequest": "700",
+ "tests": {
+ "test": {
+ "metrics": {"FrameRate": { "current": [[[0, 4], [100, 5], [205, 3]]] }}
+ },
+ },
+ "revisions": {
+ "macOS": {
+ "revision": "10.8.2 12C60",
+ "timestamp": '2018-09-27T09:49:52.670499Z',
+ },
+ "WebKit": {
+ "revision": "141977",
+ "timestamp": '2018-09-27T09:49:52.670999Z',
+ }
+ }
+ };
+
+ it("commit time of commits from measurement set queried by analysis task should match the one from `/api/commits`", async () => {
+ const expectedWebKitCommitTime = 1538041792671;
+ const expectedMacOSCommitTime = 1538041792670;
+ const remote = TestServer.remoteAPI();
+ await MockData.addMockData(TestServer.database());
+ let response = await reportAfterAddingBuilderAndAggregatorsWithResponse(reportWithCommitsNeedsRoundCommitTimeAndBuildRequest);
+ assert.equal(response['status'], 'OK');
+
+ const rawWebKitCommit = await remote.getJSONWithStatus(`/api/commits/WebKit/141977`);
+ assert.equal(rawWebKitCommit.commits[0].time, expectedWebKitCommitTime);
+
+ const rawMacOSCommit = await remote.getJSONWithStatus(`/api/commits/macOS/10.8.2%2012C60`);
+ assert.equal(rawMacOSCommit.commits[0].time, expectedMacOSCommitTime);
+
+ response = await TestServer.remoteAPI().getJSONWithStatus('/api/measurement-set/?analysisTask=500');
+ assert.equal(response['status'], 'OK');
+ assert.deepEqual(response['measurements'], [[1, 4, 3, 12, 50, [
+ ['1', '9', '10.8.2 12C60', null, expectedMacOSCommitTime], ['2', '11', '141977', null, expectedWebKitCommitTime]],
+ 1, 1362046323388, '123', 1, 1, 'current']]);
+ });
+
+ const reportWithCommitsNeedsRoundCommitTime = {
+ "buildNumber": "123",
+ "buildTime": "2013-02-28T10:12:03.388304",
+ "builderName": "someBuilder",
+ "builderPassword": "somePassword",
+ "platform": "Mountain Lion",
+ "tests": {
+ "test": {
+ "metrics": {"FrameRate": { "current": [[[0, 4], [100, 5], [205, 3]]] }}
+ },
+ },
+ "revisions": {
+ "macOS": {
+ "revision": "10.8.2 12C60",
+ "timestamp": '2018-09-27T09:49:52.670499Z',
+ },
+ "WebKit": {
+ "revision": "141977",
+ "timestamp": '2018-09-27T09:49:52.670999Z',
+ }
+ }
+ };
+
+ it("commit time of commits from measurement set queried by platform and metric should match the one from `/api/commits`", async () => {
+ const expectedWebKitCommitTime = 1538041792671;
+ const expectedMacOSCommitTime = 1538041792670;
+ const remote = TestServer.remoteAPI();
+ await addBuilderForReport(reportWithCommitsNeedsRoundCommitTime);
+ await remote.postJSON('/api/report/', [reportWithCommitsNeedsRoundCommitTime]);
+
+ const rawWebKitCommit = await remote.getJSONWithStatus(`/api/commits/WebKit/141977`);
+ assert.equal(rawWebKitCommit.commits[0].time, expectedWebKitCommitTime);
+
+ const rawMacOSCommit = await remote.getJSONWithStatus(`/api/commits/macOS/10.8.2%2012C60`);
+ assert.equal(rawMacOSCommit.commits[0].time, expectedMacOSCommitTime);
+
+ const result = await queryPlatformAndMetric('Mountain Lion', 'FrameRate');
+ const primaryCluster = await remote.getJSONWithStatus(`/api/measurement-set/?platform=${result.platformId}&metric=${result.metricId}`);
+ assert.deepEqual(primaryCluster.configurations.current, [[1, 4, 3, 12, 50, false, [
+ [1, 1, '10.8.2 12C60', null, expectedMacOSCommitTime], [2, 2, '141977', null, expectedWebKitCommitTime]],
+ expectedWebKitCommitTime, 1, 1362046323388, '123', 1]]);
+ });
});
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
