Attachment 342028 Details for Bug 186062 – Patch

[patch] Patch

bug-186062-20180605211119.patch (text/plain), 10.09 KB, created by dewei_zhu on 2018-06-05 21:11:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2018-06-05 21:11:19 PDT

Size: 10.09 KB

patch

obsolete

>Subversion Revision: 232526
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index c9543ff08dcf9e3c85a170f95e2b4ee3e8863c06..5da883df4e57c13c21f668f032a987d383bdd736 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-05-23  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         OSBuildFetcher should respect maxRevision while finding OS builds to report.
>diff --git a/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js b/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
>index c99bdc3087af3ee953c8028878c1982b16f7ae9a..d1be75046c5de7b1dff5993638e68becb3ec70e2 100644
>--- a/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
>+++ b/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
>@@ -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()) {
>@@ -49,7 +50,7 @@ class CommitSetRangeBisector {
>             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();
>+            }
>             return 0;
>         });
> 
>diff --git a/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js b/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
>index bdae30efcff197b16c3da0df065684e3c935929e..5f3783f2ee0d81033a0fa9c112feecfdc4b2d1c7 100644
>--- a/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
>+++ b/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
>@@ -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];

Flags:

rniwa: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 186062: 341522 | 342028 | 342035