REGRESSION: Searching commits can highlight wrong data points
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 19:43:45 +0000 (19:43 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 19:43:45 +0000 (19:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143272

Reviewed by Antti Koivisto.

The bug was caused by /api/commits returning commit times with millisecond precision whereas /api/runs
return commit times with only second precision. This resulted in the frontend code to match a commit
with the data point that included the next commit when the millisecond component of commit's timestamp
wasn't identically 0.

This discrepancy was caused by the fact PHP's strtotime only ignores milliseconds and /api/commits
was returning timestamp as string instead of parsing via Database::to_js_time as done in /api/runs
so miliseconds component was only preserved in /api/commits.

Fixed the bug by always using Database::to_js_time to return commit time. Also fixed to_js_time so that
it returns time in milisecond precision.

* public/api/commits.php:
(fetch_commits_between): Use Database::to_js_time for format commit times.
(format_commit): Ditto.
* public/include/db.php:
(Database::to_js_time): Parse and append millisecond component. Ignore sub-milliseconds for simplicity.
* public/v2/data.js:
(CommitLogs.fetchForTimeRange): The commit time is now an integer so don't call "replace" on it.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/api/commits.php
Websites/perf.webkit.org/public/include/db.php
Websites/perf.webkit.org/public/v2/data.js

index bf4c3a3..ee150b3 100644 (file)
@@ -1,5 +1,32 @@
 2015-03-31  Ryosuke Niwa  <rniwa@webkit.org>
 
+        REGRESSION: Searching commits can highlight wrong data points
+        https://bugs.webkit.org/show_bug.cgi?id=143272
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by /api/commits returning commit times with millisecond precision whereas /api/runs
+        return commit times with only second precision. This resulted in the frontend code to match a commit
+        with the data point that included the next commit when the millisecond component of commit's timestamp
+        wasn't identically 0.
+
+        This discrepancy was caused by the fact PHP's strtotime only ignores milliseconds and /api/commits
+        was returning timestamp as string instead of parsing via Database::to_js_time as done in /api/runs
+        so miliseconds component was only preserved in /api/commits.
+
+        Fixed the bug by always using Database::to_js_time to return commit time. Also fixed to_js_time so that
+        it returns time in milisecond precision.
+
+        * public/api/commits.php:
+        (fetch_commits_between): Use Database::to_js_time for format commit times.
+        (format_commit): Ditto.
+        * public/include/db.php:
+        (Database::to_js_time): Parse and append millisecond component. Ignore sub-milliseconds for simplicity.
+        * public/v2/data.js:
+        (CommitLogs.fetchForTimeRange): The commit time is now an integer so don't call "replace" on it.
+
+2015-03-31  Ryosuke Niwa  <rniwa@webkit.org>
+
         Perf dashboard should show relative change in values
         https://bugs.webkit.org/show_bug.cgi?id=143252
 
index d1d79a3..597f716 100644 (file)
@@ -98,6 +98,8 @@ function fetch_commits_between($db, $repository_id, $first, $second, $keyword =
     $commits = $db->query_and_fetch_all($statements . ' ORDER BY commit_time', $values);
     if (!is_array($commits))
         exit_with_error('FailedToFetchCommits', array('repository' => $repository_id, 'first' => $first, 'second' => $second));
+    foreach ($commits as &$commit)
+        $commit['time'] = Database::to_js_time($commit['time']);
     return $commits;
 }
 
@@ -106,7 +108,7 @@ function format_commit($commit_row, $committer_row) {
         'id' => $commit_row['commit_id'],
         'revision' => $commit_row['commit_revision'],
         'parent' => $commit_row['commit_parent'],
-        'time' => $commit_row['commit_time'],
+        'time' => Database::to_js_time($commit_row['commit_time']),
         'authorName' => $committer_row ? $committer_row['committer_name'] : null,
         'authorEmail' => $committer_row ? $committer_row['committer_account'] : null,
         'message' => $commit_row['commit_message']
index 1917c09..35b5e29 100644 (file)
@@ -65,7 +65,11 @@ class Database
     }
 
     static function to_js_time($time_str) {
-        return strtotime($time_str) * 1000;
+        $timestamp_in_s = strtotime($time_str);
+        $dot_index = strrpos($time_str, '.');
+        if ($dot_index !== FALSE)
+            $timestamp_in_s += floatval(substr($time_str, $dot_index));
+        return intval($timestamp_in_s * 1000);
     }
 
     function connect() {
index a6e4af6..dbf86a9 100755 (executable)
@@ -84,7 +84,7 @@ CommitLogs.fetchForTimeRange = function (repository, from, to, keyword)
             }
 
             var fetchedCommits = data.commits;
-            fetchedCommits.forEach(function (commit) { commit.time = new Date(commit.time.replace(' ', 'T')); });
+            fetchedCommits.forEach(function (commit) { commit.time = new Date(commit.time); });
 
             if (useCache)
                 CommitLogs._cacheConsecutiveCommits(repository, from, to, fetchedCommits);