CommitSet range bisector should use commits occur in commit sets which specify the range as valid commits for commits without ordering.

• Component: New Bugs
• Status: NEW ---

CommitSet range bisector should use commits occur in commit sets which specify the range as valid commits for commits without ordering.

Created attachment 341522 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 341522 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341522&action=review

> Websites/perf.webkit.org/ChangeLog:8
> +        For commits without ordering, we should use the commits occurs in the commit sets which specify the range as valid commits.

What do you mean by "the commits occurs in the commit sets"?

Comment on attachment 341522 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341522&action=review

>> Websites/perf.webkit.org/ChangeLog:8
>> +        For commits without ordering, we should use the commits occurs in the commit sets which specify the range as valid commits.
> 
> What do you mean by "the commits occurs in the commit sets"?

Let's say we cannot order commits for WebKit (r100, r102) due to lack of commit time and commit order. Then we should make sure any commit set get chosen, the commit for WebKit should be either r100 or r102.

[Ryosuke Niwa]

Comment on attachment 341522 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341522&action=review

>>> Websites/perf.webkit.org/ChangeLog:8
>>> +        For commits without ordering, we should use the commits occurs in the commit sets which specify the range as valid commits.
>> 
>> What do you mean by "the commits occurs in the commit sets"?
> 
> Let's say we cannot order commits for WebKit (r100, r102) due to lack of commit time and commit order. Then we should make sure any commit set get chosen, the commit for WebKit should be either r100 or r102.

We should probably order lexicologically by the revision / commit hash / version number in that case. 
If the order changes from time to time, that could spell a disaster.

Created attachment 342028 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 342028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342028&action=review

r=me assuming you fix the comparator & add a unit test.

> Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js:104
> +                return firstCommit.revision() < secondCommit.revision();

This can't be write. We need to be returning 0, -1, or 1.
Please add a test case which catches this bug.

[EWS Watchlist]

Comment on attachment 342028 [details]
Patch

Attachment 342028 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/8026194

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html

[EWS Watchlist]

Created attachment 342035 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit