CommitSet range bisector should use commits occur in commit sets which specify the...
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jun 2018 18:23:17 +0000 (18:23 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jun 2018 18:23:17 +0000 (18:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186062

Reviewed by Ryosuke Niwa.

For commits without ordering, we should use the commits occurs in the commit sets which specify the range as valid commits.
Commit sets in range should only contain those valid commits for corresponding repositories.

* public/v3/commit-set-range-bisector.js: Updated logic to add check on commits without ordering.
(CommitSetRangeBisector.async.commitSetClosestToMiddleOfAllCommits):
* unit-tests/commit-set-range-bisector-tests.js: Added a unit test for this case.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js

index 0a3c0e0..d533507 100644 (file)
@@ -1,3 +1,17 @@
+2018-05-29  Dewei Zhu  <dewei_zhu@apple.com>
+
+        CommitSet range bisector should use commits occur in commit sets which specify the range as valid commits for commits without ordering.
+        https://bugs.webkit.org/show_bug.cgi?id=186062
+
+        Reviewed by Ryosuke Niwa.
+
+        For commits without ordering, we should use the commits occurs in the commit sets which specify the range as valid commits.
+        Commit sets in range should only contain those valid commits for corresponding repositories.
+
+        * public/v3/commit-set-range-bisector.js: Updated logic to add check on commits without ordering.
+        (CommitSetRangeBisector.async.commitSetClosestToMiddleOfAllCommits):
+        * unit-tests/commit-set-range-bisector-tests.js: Added a unit test for this case.
+
 2018-06-07  Dewei Zhu  <dewei_zhu@apple.com>
 
         Related task may not have a metric or platform.
index c99bdc3..5840d68 100644 (file)
@@ -16,18 +16,19 @@ class CommitSetRangeBisector {
         const commitRangeByRepository = new Map;
         const indexForAllTimelessCommitsWithOrderByRepository = new Map;
         const allCommitsWithCommitTime = [];
-        const topLevelRepositoriesWithOrderedCommits = firstCommitSet.topLevelRepositories()
-            .filter((repository) => {
-                const firstCommit = firstCommitSet.commitForRepository(repository);
-                const secondCommit = secondCommitSet.commitForRepository(repository);
-                return CommitLog.hasOrdering(firstCommit, secondCommit);
-            });
+        const repositoriesWithoutOrdering = [];
 
-        await Promise.all(topLevelRepositoriesWithOrderedCommits.map(async (repository) => {
+        await Promise.all(firstCommitSet.topLevelRepositories().map(async (repository) => {
             const firstCommit = firstCommitSet.commitForRepository(repository);
             const secondCommit = secondCommitSet.commitForRepository(repository);
-            const [startCommit, endCommit] = CommitLog.orderTwoCommits(firstCommit, secondCommit);
 
+            if (!CommitLog.hasOrdering(firstCommit, secondCommit)) {
+                repositoriesWithoutOrdering.push(repository);
+                commitRangeByRepository.set((repository), (commit) => commit === firstCommit || commit === secondCommit);
+                return;
+            }
+
+            const [startCommit, endCommit] = CommitLog.orderTwoCommits(firstCommit, secondCommit);
             const commits = startCommit === endCommit ? [startCommit] : await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision());
 
             if (startCommit.hasCommitTime()) {
@@ -45,11 +46,11 @@ class CommitSetRangeBisector {
             }
         }));
 
-        if (!repositoriesWithCommitTime.size && !indexForAllTimelessCommitsWithOrderByRepository.size)
+        if (!repositoriesWithCommitTime.size && !indexForAllTimelessCommitsWithOrderByRepository.size && !repositoriesWithoutOrdering.size)
             return null;
 
         const commitSetsInRange = this._findCommitSetsWithinRange(firstCommitSet, secondCommitSet, availableCommitSets, commitRangeByRepository);
-        let sortedCommitSets = this._orderCommitSetsByTimeAndOrderThenDeduplicate(commitSetsInRange, repositoriesWithCommitTime, [...indexForAllTimelessCommitsWithOrderByRepository.keys()]);
+        let sortedCommitSets = this._orderCommitSetsByTimeAndOrderThenDeduplicate(commitSetsInRange, repositoriesWithCommitTime, [...indexForAllTimelessCommitsWithOrderByRepository.keys()], repositoriesWithoutOrdering);
         if (!sortedCommitSets.length)
             return null;
 
@@ -76,7 +77,7 @@ class CommitSetRangeBisector {
         });
     }
 
-    static _orderCommitSetsByTimeAndOrderThenDeduplicate(commitSets, repositoriesWithCommitTime, repositoriesWithCommitOrderOnly)
+    static _orderCommitSetsByTimeAndOrderThenDeduplicate(commitSets, repositoriesWithCommitTime, repositoriesWithCommitOrderOnly, repositoriesWithoutOrdering)
     {
         const sortedCommitSets = commitSets.sort((firstCommitSet, secondCommitSet) => {
             for (const repository of repositoriesWithCommitTime) {
@@ -95,6 +96,13 @@ class CommitSetRangeBisector {
                     continue;
                 return diff;
             }
+            for (const repository of repositoriesWithoutOrdering) {
+                const firstCommit = firstCommitSet.commitForRepository(repository);
+                const secondCommit = secondCommitSet.commitForRepository(repository);
+                if (firstCommit === secondCommit)
+                    continue;
+                return firstCommit.revision() < secondCommit.revision() ? -1 : 1;
+            }
             return 0;
         });
 
index bdae30e..1e016d6 100644 (file)
@@ -279,6 +279,42 @@ describe('CommitSetRangeBisector', () => {
         ];
     }
 
+    function commitSetWithOnlySomeCommitsHaveOrdering()
+    {
+        return [
+            CommitSet.ensureSingleton(23, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(111, MockModels.osx, 'osx-commit-111', 0), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(24, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(112, MockModels.osx, 'osx-commit-112', 0), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(25, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(113, MockModels.osx, 'osx-commit-113', 0), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(26, {
+                revisionItems: [
+                    { commit: makeCommit(3, MockModels.webkit, 'webkit-commit-3', 30), requiresBuild: false },
+                    { commit: makeCommit(114, MockModels.osx, 'osx-commit-114', 0), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(27, {
+                revisionItems: [
+                    { commit: makeCommit(6, MockModels.webkit, 'webkit-commit-6', 60), requiresBuild: false },
+                    { commit: makeCommit(113, MockModels.osx, 'osx-commit-113', 0), requiresBuild: false }
+                ],
+                customRoots: []}),
+        ];
+    }
+
     function createRoot()
     {
         return UploadedFile.ensureSingleton(456, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
@@ -832,6 +868,59 @@ describe('CommitSetRangeBisector', () => {
             assert.equal(await promise, allCommitSets[4]);
         });
 
+        it('should filter out commits those are not in any commit sets for commit without ordering', async () => {
+            const allCommitSets = commitSetWithOnlySomeCommitsHaveOrdering();
+            const startCommitSet = allCommitSets[0];
+            const endCommitSet = allCommitSets[allCommitSets.length - 1];
+            const promise = CommitSetRangeBisector.commitSetClosestToMiddleOfAllCommits([startCommitSet, endCommitSet], allCommitSets);
+            const webkitId = MockModels.webkit.id();
+            const osxId = MockModels.osx.id();
+
+            assert.equal(requests.length, 1);
+            assert.equal(requests[0].url, '/api/commits/11/?precedingRevision=webkit-commit-1&lastRevision=webkit-commit-6');
+
+            requests[0].resolve({
+                'commits': [
+                    {
+                        repository: webkitId,
+                        id: 2,
+                        revision: 'webkit-commit-2',
+                        ownsCommits: false,
+                        time: 20
+                    },
+                    {
+                        repository: webkitId,
+                        id: 3,
+                        revision: 'webkit-commit-3',
+                        ownsCommits: false,
+                        time: 30
+                    },
+                    {
+                        repository: webkitId,
+                        id: 4,
+                        revision: 'webkit-commit-4',
+                        ownsCommits: false,
+                        time: 40
+                    },
+                    {
+                        repository: webkitId,
+                        id: 5,
+                        revision: 'webkit-commit-5',
+                        ownsCommits: false,
+                        time: 50
+                    },
+                    {
+                        repository: webkitId,
+                        id: 6,
+                        revision: 'webkit-commit-6',
+                        ownsCommits: false,
+                        time: 60
+                    },
+                ]
+            });
+            assert.equal(await promise, allCommitSets[2]);
+        });
+
         it('should still work even some commits do not monotonically increasing', async () => {
             const allCommitSets = commitSetsWithSomeCommitsNotMonotonicallyIncrease();
             const startCommitSet = allCommitSets[0];
@@ -899,4 +988,26 @@ describe('CommitSetRangeBisector', () => {
             assert.equal(await promise, allCommitSets[3]);
         });
     });
+
+    describe('_orderCommitSetsByTimeAndOrderThenDeduplicate', () => {
+        it('should sort by alphabetically for commits without ordering', () => {
+            const oneCommitSet = CommitSet.ensureSingleton(100, {
+                revisionItems: [
+                    { commit: makeCommit(1001, MockModels.webkit, 'webkit-commit-1001'), requiresBuild: false}
+                ],
+                customRoots: []
+            });
+
+            const anotherCommitSet = CommitSet.ensureSingleton(101, {
+                revisionItems: [
+                    { commit: makeCommit(1002, MockModels.webkit, 'webkit-commit-1002'), requiresBuild: false}
+                ],
+                customRoots: []
+            });
+
+            const [commitSet0, commitSet1] = CommitSetRangeBisector._orderCommitSetsByTimeAndOrderThenDeduplicate([anotherCommitSet, oneCommitSet], [], [], [MockModels.webkit]);
+            assert.equal(oneCommitSet, commitSet0);
+            assert.equal(anotherCommitSet, commitSet1);
+        })
+    })
 });
\ No newline at end of file