[Findit] Use results from heuristic analysis to do faster culprit finding.

[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile runs compile at 5 revisions r1, r2, r3, r4, and r5 in order to identify the culprit r5.

With the heuristics-based results, the recipe could divide the entire regression range into two smaller sub-ranges:
  sub-range1: r4, and [r5, r6, r7, r8, r9, r10]
  sub-range2: r0, and [r1, r2, r3]
The recipe checks sub-range1 first, and it runs compile only at 2 revision r4 (pass) and r5 (fail) in order to identify the culprit r5. The second sub-range is skipped because of the heuristics-based results.


BUG=596967
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299866

[stgao, 2016-04-07 23:01:20]

Patchset #1 (id:1) has been deleted

[stgao, 2016-04-07 23:13:10]

Description was changed from

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

BUG=596967
==========

to

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs at 5 revisions r1, r2, r3, r4, and r5 to identify the culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller ranges:
  range1: r4, and [r5, r6, r7, r8, r9, r10]
  range2: r0, and [r1, r2, r3]
The recipe checks range1 first, and it runs compile only at 2 revision r4 (pass)
and then r5 (fail) to identify the culprit r5.


BUG=596967
==========

[stgao, 2016-04-07 23:14:22]

Description was changed from

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs at 5 revisions r1, r2, r3, r4, and r5 to identify the culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller ranges:
  range1: r4, and [r5, r6, r7, r8, r9, r10]
  range2: r0, and [r1, r2, r3]
The recipe checks range1 first, and it runs compile only at 2 revision r4 (pass)
and then r5 (fail) to identify the culprit r5.


BUG=596967
==========

to

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs at 5 revisions r1, r2, r3, r4, and r5 to identify the culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller ranges:
  range1: r4, and [r5, r6, r7, r8, r9, r10]
  range2: r0, and [r1, r2, r3]
The recipe checks range1 first, and it runs compile only at 2 revision r4 (pass)
and then r5 (fail) to identify the culprit r5. The second sub range is skipped
because of the heuristics-based results.


BUG=596967
==========

[stgao, 2016-04-08 00:31:09]

Patchset #1 (id:20001) has been deleted

[stgao, 2016-04-08 00:37:42]

Patchset #1 (id:40001) has been deleted

[stgao, 2016-04-08 00:38:47]

stgao@chromium.org changed reviewers:
+ lijeffrey@chromium.org

[lijeffrey, 2016-04-11 23:52:58]

lgtm with nits

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:127: # order from older one to
newer one.
nit: in order from oldest to newest.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:139: #   sub-range2: r4 and
[r5, r6, r7]
nit: shouldn't this be r4 and [r5, r6]?

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:142: # Sub-ranges with new
revisions are checked first, because a compile failure
per discussion, maybe include a comment how if there are 2 suspected CLs its is
more common than not that it is due to 2 conflicting changes, in which case the
later commit is the triggering culprit which is why we check the later one
first.

The comment as it is sounds like this is a corner case, when it's actually the
general case.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:167: # * if r1 is the actual
culprit, we check r7 and r4 before checking the
Question: So in this case, this is the worst, because we check r7, find it is
red, then have to go back to r4 (with no incremental compile benefit), then find
it's also red, and have to start over from r1 (again with no incremental compile
benefit), and possibly even worse it's actually r3? (I'll follow up with Dan to
see if artifacts from r0 are in fact cached somewhere and if we can access it it
should mitigate this problem even if we have to do r7 then r4 then r1)

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:170: suspected_revision_index 
= [
nit: remove 1 space before =

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:217: # is the first revision of
previous range as identified above.
is this comment always true?

for example in our case, we have r5 and r8 as suspects, so r7 is the first
revision. If r7 passes, doesn't that mean the culprit should be in [r8, r9,
r10], but "or is the first revision of previous range" suggests it's also
possible it's r4?

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:225: # First failure after a
serial of pass.
nit: a series of passes

[stgao, 2016-04-12 00:07:51]

Description was changed from

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs at 5 revisions r1, r2, r3, r4, and r5 to identify the culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller ranges:
  range1: r4, and [r5, r6, r7, r8, r9, r10]
  range2: r0, and [r1, r2, r3]
The recipe checks range1 first, and it runs compile only at 2 revision r4 (pass)
and then r5 (fail) to identify the culprit r5. The second sub range is skipped
because of the heuristics-based results.


BUG=596967
==========

to

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs compile at 5 revisions r1, r2, r3, r4, and r5 in order to identify the
culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller sub-ranges:
  sub-range1: r4, and [r5, r6, r7, r8, r9, r10]
  sub-range2: r0, and [r1, r2, r3]
The recipe checks sub-range1 first, and it runs compile only at 2 revision r4
(pass) and r5 (fail) in order to identify the culprit r5. The second sub-range
is skipped because of the heuristics-based results.


BUG=596967
==========

[stgao, 2016-04-12 01:29:27]

Patchset #2 (id:80001) has been deleted

[stgao, 2016-04-12 01:29:56]

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:127: # order from older one to
newer one.
On 2016/04/11 23:52:58, lijeffrey wrote:
> nit: in order from oldest to newest.

Done.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:139: #   sub-range2: r4 and
[r5, r6, r7]
On 2016/04/11 23:52:58, lijeffrey wrote:
> nit: shouldn't this be r4 and [r5, r6]?

Good catch.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:142: # Sub-ranges with new
revisions are checked first, because a compile failure
On 2016/04/11 23:52:58, lijeffrey wrote:
> per discussion, maybe include a comment how if there are 2 suspected CLs its
is
> more common than not that it is due to 2 conflicting changes, in which case
the
> later commit is the triggering culprit which is why we check the later one
> first.
> 
> The comment as it is sounds like this is a corner case, when it's actually the
> general case.

Done. Good idea.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:167: # * if r1 is the actual
culprit, we check r7 and r4 before checking the
On 2016/04/11 23:52:58, lijeffrey wrote:
> Question: So in this case, this is the worst, because we check r7, find it is
> red, then have to go back to r4 (with no incremental compile benefit), then
find
> it's also red, and have to start over from r1 (again with no incremental
compile
> benefit), and possibly even worse it's actually r3? (I'll follow up with Dan
to
> see if artifacts from r0 are in fact cached somewhere and if we can access it
it
> should mitigate this problem even if we have to do r7 then r4 then r1)

If the failure is a CC or CXX compile failure, it won't be that worse, because 
usually only 2-4k objects need compiling.
For link failure, the worst case here might be an issue, because much more
objects need compiling. We could check how it goes in actual runs and then come
up with better solution in such case.

The cached artifacts from r0 will always be helpful in either case, especially
when the slave is not up-to-date enough.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:170: suspected_revision_index 
= [
On 2016/04/11 23:52:58, lijeffrey wrote:
> nit: remove 1 space before =

Done.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:217: # is the first revision of
previous range as identified above.
On 2016/04/11 23:52:58, lijeffrey wrote:
> is this comment always true?
> 
> for example in our case, we have r5 and r8 as suspects, so r7 is the first
> revision. If r7 passes, doesn't that mean the culprit should be in [r8, r9,
> r10], but "or is the first revision of previous range" suggests it's also
> possible it's r4?

Sorry for the confusion. I updated the wording and also the example above.

We test [r7,..., r10] before [r4, ..., r6].
If r7 passes, the culprit is in [r8, ..., r10].
If r7 fails and r4 passes, r7 might be the culprit. And this is the case
described here.

https://codereview.chromium.org/1869223002/diff/60001/scripts/slave/recipes/f...
scripts/slave/recipes/findit/chromium/compile.py:225: # First failure after a
serial of pass.
On 2016/04/11 23:52:58, lijeffrey wrote:
> nit: a series of passes

Done.

[lijeffrey, 2016-04-12 08:41:53]

lgtm

https://codereview.chromium.org/1869223002/diff/100001/scripts/slave/recipes/...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/1869223002/diff/100001/scripts/slave/recipes/...
scripts/slave/recipes/findit/chromium/compile.py:133: # before a suspected
revision and then at the suspected revision itself. And a
nit: And is not necessary, "A sub-range starts ..." is fine.

[stgao, 2016-04-12 18:29:00]

https://codereview.chromium.org/1869223002/diff/100001/scripts/slave/recipes/...
File scripts/slave/recipes/findit/chromium/compile.py (right):

https://codereview.chromium.org/1869223002/diff/100001/scripts/slave/recipes/...
scripts/slave/recipes/findit/chromium/compile.py:133: # before a suspected
revision and then at the suspected revision itself. And a
On 2016/04/12 08:41:52, lijeffrey wrote:
> nit: And is not necessary, "A sub-range starts ..." is fine.

Done.

[stgao, 2016-04-12 18:29:06]

The CQ bit was checked by stgao@chromium.org

[stgao, 2016-04-12 18:29:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
lijeffrey@chromium.org
Link to the patchset: https://codereview.chromium.org/1869223002/#ps120001
(title: "Fix nit.")

[commit-bot: I haz the power, 2016-04-12 18:29:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1869223002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1869223002/120001

[commit-bot: I haz the power, 2016-04-12 18:33:09]

Description was changed from

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs compile at 5 revisions r1, r2, r3, r4, and r5 in order to identify the
culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller sub-ranges:
  sub-range1: r4, and [r5, r6, r7, r8, r9, r10]
  sub-range2: r0, and [r1, r2, r3]
The recipe checks sub-range1 first, and it runs compile only at 2 revision r4
(pass) and r5 (fail) in order to identify the culprit r5. The second sub-range
is skipped because of the heuristics-based results.


BUG=596967
==========

to

==========
[Findit] Use results from heuristic analysis to do faster culprit finding.

The goal is to make Findit run less rounds of compile and as a result speeds up
the culprit finding.

An example of how this works.
Case:
  Entire regression range is: (r0, r10].
  Heuristics-based results are: [r5]
  Actual culprit is: r5

Currently without heuristics-based results, the recipe findit/chromium/compile
runs compile at 5 revisions r1, r2, r3, r4, and r5 in order to identify the
culprit r5.

With the heuristics-based results, the recipe could divide the entire regression
range into two smaller sub-ranges:
  sub-range1: r4, and [r5, r6, r7, r8, r9, r10]
  sub-range2: r0, and [r1, r2, r3]
The recipe checks sub-range1 first, and it runs compile only at 2 revision r4
(pass) and r5 (fail) in order to identify the culprit r5. The second sub-range
is skipped because of the heuristics-based results.


BUG=596967

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299866
==========

[commit-bot: I haz the power, 2016-04-12 18:33:11]

Committed patchset #3 (id:120001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299866