Make a directory in which to put bisect-related modules.

Make a directory in which to put bisect-related modules.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282209

[qyearsley, 2014-06-27 17:50:06]

For the near-term, I'd still like to split off parts of
bisect-perf-regression.py into separate modules and put all of these modules in
a directory.

[prasadv, 2014-06-27 17:56:42]

We should also consider changing references to these files in other modules.

https://code.google.com/p/chromium/codesearch#search/&q=bisect-perf-regressio...

[qyearsley, 2014-06-27 17:56:49]

Any thoughts about what the directory for bisect-related modules should be
called?

Is "bisect" okay? Other ideas include "bisect_tool" and "bisect_perf".

Are there any potentially Python import problems?

Also, just now when I tried to run python
src/tools/bisect-perf-regression_test.py, I got an error in
bisect-perf-regression.py on line 746 in the expression "if 'ninja' in
os.getenv('GYP_GENERATORS')", since I didn't have that environment variable
station on my local workstation; I assume that this is unrelated to this CL.

There are some pylint warnings, but no import-related pylint warnings.

[tonyg, 2014-06-27 17:56:51]

lgtm++

I suggest this patch include OWNERS and README in the new directory. Initial
owners might be you, prasad, simon and me? The README should explain the bisect
tool and link to a dev.chromium.org page describing overall design, how it is
used, etc (writing that up need not block this patch).

[shatch, 2014-06-27 17:59:59]

Oooh thanks for doing this, lgtm!

[tonyg, 2014-06-27 18:01:44]

On 2014/06/27 17:56:49, qyearsley wrote:
> Any thoughts about what the directory for bisect-related modules should be
> called?
> 
> Is "bisect" okay? Other ideas include "bisect_tool" and "bisect_perf".

I like "bisect". It keeps it general and allows the tool to expand to handle all
bisect needs. We used AutoBisect as the name for the crbug component. So this
might also be a good chance to think whether we want to run with that branding
for the project or come up with another name for the product.

> There are some pylint warnings, but no import-related pylint warnings.

A good next step would be copying parts of PRESUBMIT.py from tools/perf to this
directory. And eventually we should move that up to tools/ (although not sure
how hard it'd be to make it green).

Finally, I hope to see unittests here soon. We should think about the best way
to get those running as tree closers as well.

[qyearsley, 2014-06-27 18:08:07]

On 2014/06/27 17:56:51, tonyg wrote:
> I suggest this patch include OWNERS and README in the new directory. Initial
> owners might be you, prasad, simon and me? The README should explain the
bisect
> tool and link to a http://dev.chromium.org page describing overall design, how
it is
> used, etc (writing that up need not block this patch).

OWNERS and README are a good idea; run-bisect-perf-regression.py and
bisect-perf-regression.py have some info in the headers for those files, so the
README may be short-ish, although it should at least list what these different
files are and what they're for.

Question: Should bisect-perf-regression.py (and run-bisect-perf-regression.py,
run-bisect-perf-regression.cfg, etc.) also be in this directory? Or should those
remain in src/tools? Maybe "run-bisect-perf-regression.py,
prepare-bisect-perf-regression.py, run-bisect-perf-regression.cfg should be in
src/tools but bisect-perf-regression.py should be moved into this new directory
and re-named?

[tonyg, 2014-06-27 18:17:23]

On 2014/06/27 18:08:07, qyearsley wrote:
> On 2014/06/27 17:56:51, tonyg wrote:
> > I suggest this patch include OWNERS and README in the new directory. Initial
> > owners might be you, prasad, simon and me? The README should explain the
> bisect
> > tool and link to a http://dev.chromium.org page describing overall design,
how
> it is
> > used, etc (writing that up need not block this patch).
> 
> OWNERS and README are a good idea; run-bisect-perf-regression.py and
> bisect-perf-regression.py have some info in the headers for those files, so
the
> README may be short-ish, although it should at least list what these different
> files are and what they're for.
> 
> Question: Should bisect-perf-regression.py (and run-bisect-perf-regression.py,
> run-bisect-perf-regression.cfg, etc.) also be in this directory? Or should
those
> remain in src/tools? Maybe "run-bisect-perf-regression.py,
> prepare-bisect-perf-regression.py, run-bisect-perf-regression.cfg should be in
> src/tools but bisect-perf-regression.py should be moved into this new
directory
> and re-named?

I think everything should be moved into this dir eventually. tools/ itself is
generic enough that I think it should only contain subdirs.

[chromium-reviews, 2014-06-27 18:24:37]

+1 for moving everything related to bisect into new directory.
Also since we are planning expand this to support functional tests, should
we rename bisect-perf-regression and other filename referring
*bisect-perf-regression to something like "bisect_regression"?


On Fri, Jun 27, 2014 at 11:17 AM, <tonyg@chromium.org> wrote:

> On 2014/06/27 18:08:07, qyearsley wrote:
>
>> On 2014/06/27 17:56:51, tonyg wrote:
>> > I suggest this patch include OWNERS and README in the new directory.
>> Initial
>> > owners might be you, prasad, simon and me? The README should explain the
>> bisect
>> > tool and link to a http://dev.chromium.org page describing overall
>> design,
>>
> how
>
>> it is
>> > used, etc (writing that up need not block this patch).
>>
>
>  OWNERS and README are a good idea; run-bisect-perf-regression.py and
>> bisect-perf-regression.py have some info in the headers for those files,
>> so
>>
> the
>
>> README may be short-ish, although it should at least list what these
>> different
>> files are and what they're for.
>>
>
>  Question: Should bisect-perf-regression.py (and
>> run-bisect-perf-regression.py,
>> run-bisect-perf-regression.cfg, etc.) also be in this directory? Or
>> should
>>
> those
>
>> remain in src/tools? Maybe "run-bisect-perf-regression.py,
>> prepare-bisect-perf-regression.py, run-bisect-perf-regression.cfg should
>> be in
>> src/tools but bisect-perf-regression.py should be moved into this new
>>
> directory
>
>> and re-named?
>>
>
> I think everything should be moved into this dir eventually. tools/ itself
> is
> generic enough that I think it should only contain subdirs.
>
> https://codereview.chromium.org/359013002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[shatch, 2014-06-27 18:31:18]

I could imagine bisect-perf-regression getting split up into a bunch of separate
files (ie. builder/bisect_runner/misc) and bisect-perf-regression.py can
disappear entirely. 

I don't think anything beyond the various run-bisect flavours refer to
bisect-perf-regression, so that can probably be moved immediately or in the near
future to the bisect directory. 

On 2014/06/27 18:24:37, chromium-reviews wrote:
> +1 for moving everything related to bisect into new directory.
> Also since we are planning expand this to support functional tests, should
> we rename bisect-perf-regression and other filename referring
> *bisect-perf-regression to something like "bisect_regression"?
> 
> 
> On Fri, Jun 27, 2014 at 11:17 AM, <mailto:tonyg@chromium.org> wrote:
> 
> > On 2014/06/27 18:08:07, qyearsley wrote:
> >
> >> On 2014/06/27 17:56:51, tonyg wrote:
> >> > I suggest this patch include OWNERS and README in the new directory.
> >> Initial
> >> > owners might be you, prasad, simon and me? The README should explain the
> >> bisect
> >> > tool and link to a http://dev.chromium.org page describing overall
> >> design,
> >>
> > how
> >
> >> it is
> >> > used, etc (writing that up need not block this patch).
> >>
> >
> >  OWNERS and README are a good idea; run-bisect-perf-regression.py and
> >> bisect-perf-regression.py have some info in the headers for those files,
> >> so
> >>
> > the
> >
> >> README may be short-ish, although it should at least list what these
> >> different
> >> files are and what they're for.
> >>
> >
> >  Question: Should bisect-perf-regression.py (and
> >> run-bisect-perf-regression.py,
> >> run-bisect-perf-regression.cfg, etc.) also be in this directory? Or
> >> should
> >>
> > those
> >
> >> remain in src/tools? Maybe "run-bisect-perf-regression.py,
> >> prepare-bisect-perf-regression.py, run-bisect-perf-regression.cfg should
> >> be in
> >> src/tools but bisect-perf-regression.py should be moved into this new
> >>
> > directory
> >
> >> and re-named?
> >>
> >
> > I think everything should be moved into this dir eventually. tools/ itself
> > is
> > generic enough that I think it should only contain subdirs.
> >
> > https://codereview.chromium.org/359013002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[qyearsley, 2014-06-27 18:42:16]

On 2014/06/27 18:31:18, shatch wrote:
> I could imagine bisect-perf-regression getting split up into a bunch of
separate
> files (ie. builder/bisect_runner/misc) and bisect-perf-regression.py can
> disappear entirely. 
> 
> I don't think anything beyond the various run-bisect flavours refer to
> bisect-perf-regression, so that can probably be moved immediately or in the
near
> future to the bisect directory. 
> 
> On 2014/06/27 18:24:37, chromium-reviews wrote:
> > +1 for moving everything related to bisect into new directory.
> > Also since we are planning expand this to support functional tests, should
> > we rename bisect-perf-regression and other filename referring
> > *bisect-perf-regression to something like "bisect_regression"?

Alright, so:
 * "bisect" is probably a reasonable name for the new directory since it will
encompass both manual (run by the end user with feedback from the user) and
automatic bisect, for both performance regressions and functional regressions.
 * All of the run-bisect* files should be moved into the directory in a
follow-up CL.
 * bisect-perf-regression.py will be broken into smaller parts, which all belong
in the new directory.

What about bisect_builds.py?

In the latest patch I added a first draft of a README file -- this README file
will become outdated pretty soon. Any suggestions for other things to put there?
Is the content of the README accurate?

[shatch, 2014-06-27 18:47:39]

On 2014/06/27 18:42:16, qyearsley wrote:
> On 2014/06/27 18:31:18, shatch wrote:
> > I could imagine bisect-perf-regression getting split up into a bunch of
> separate
> > files (ie. builder/bisect_runner/misc) and bisect-perf-regression.py can
> > disappear entirely. 
> > 
> > I don't think anything beyond the various run-bisect flavours refer to
> > bisect-perf-regression, so that can probably be moved immediately or in the
> near
> > future to the bisect directory. 
> > 
> > On 2014/06/27 18:24:37, chromium-reviews wrote:
> > > +1 for moving everything related to bisect into new directory.
> > > Also since we are planning expand this to support functional tests, should
> > > we rename bisect-perf-regression and other filename referring
> > > *bisect-perf-regression to something like "bisect_regression"?
> 
> Alright, so:
>  * "bisect" is probably a reasonable name for the new directory since it will
> encompass both manual (run by the end user with feedback from the user) and
> automatic bisect, for both performance regressions and functional regressions.
>  * All of the run-bisect* files should be moved into the directory in a
> follow-up CL.
>  * bisect-perf-regression.py will be broken into smaller parts, which all
belong
> in the new directory.
> 
> What about bisect_builds.py?
> 
> In the latest patch I added a first draft of a README file -- this README file
> will become outdated pretty soon. Any suggestions for other things to put
there?
> Is the content of the README accurate?

Believe bisect-builds is an unrelated script to bisect between archived builds
(with manual input). Mostly used by testers?

Unrelated: Should testers start using run-bisect-manual instead?

[qyearsley, 2014-06-28 00:08:30]

On 2014/06/27 18:47:39, shatch wrote:
> On 2014/06/27 18:42:16, qyearsley wrote:
> > On 2014/06/27 18:31:18, shatch wrote:
> > > I could imagine bisect-perf-regression getting split up into a bunch of
> > separate
> > > files (ie. builder/bisect_runner/misc) and bisect-perf-regression.py can
> > > disappear entirely. 
> > > 
> > > I don't think anything beyond the various run-bisect flavours refer to
> > > bisect-perf-regression, so that can probably be moved immediately or in
the
> > near
> > > future to the bisect directory. 
> > > 
> > > On 2014/06/27 18:24:37, chromium-reviews wrote:
> > > > +1 for moving everything related to bisect into new directory.
> > > > Also since we are planning expand this to support functional tests,
should
> > > > we rename bisect-perf-regression and other filename referring
> > > > *bisect-perf-regression to something like "bisect_regression"?
> > 
> > Alright, so:
> >  * "bisect" is probably a reasonable name for the new directory since it
will
> > encompass both manual (run by the end user with feedback from the user) and
> > automatic bisect, for both performance regressions and functional
regressions.
> >  * All of the run-bisect* files should be moved into the directory in a
> > follow-up CL.
> >  * bisect-perf-regression.py will be broken into smaller parts, which all
> belong
> > in the new directory.
> > 
> > What about bisect_builds.py?
> > 
> > In the latest patch I added a first draft of a README file -- this README
file
> > will become outdated pretty soon. Any suggestions for other things to put
> there?
> > Is the content of the README accurate?
> 
> Believe bisect-builds is an unrelated script to bisect between archived builds
> (with manual input). Mostly used by testers?
> 
> Unrelated: Should testers start using run-bisect-manual instead?

I've used bisect-builds before -- if it serves the same function as
run-bisect-manual-test, then maybe later bisect-builds can be removed?

So now I've tried a dry run of run-bisect-perf-regression.py locally, and it
appears to be OK. Note that if the working directory is given as src/tools, then
some things will be checked out in src/tools/bisect, but that's unlikely to be
an issue because (1) on the trybots the working directory is not given as
src/tools, and (2) no existing files are affected in that dir unless one of them
is named .gclient, .gclient_entries, or src.

So, I'll now try committing.

[qyearsley, 2014-06-28 00:08:33]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2014-06-28 00:10:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qyearsley@chromium.org/359013002/20001

[commit-bot: I haz the power, 2014-06-28 00:24:56]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/22788)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/27213)

[commit-bot: I haz the power, 2014-06-28 00:26:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-28 00:26:07]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/22799)

[qyearsley, 2014-06-28 01:39:22]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2014-06-28 01:40:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qyearsley@chromium.org/359013002/40001

[commit-bot: I haz the power, 2014-06-28 04:35:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-28 04:47:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-28 04:47:15]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[qyearsley, 2014-06-29 22:47:30]

On 2014/06/28 04:47:15, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   mac_chromium_rel on tryserver.chromium
>
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

The tests are failing because the package name "bisect" conflicts with the
standard module name "bisect" (https://docs.python.org/2/library/bisect.html).

I think it's still possible to call the module by using absolute imports ("from
__future__ import absolute_import"), but it may be simpler to just come up with
a name that doesn't conflict.

Still possible:
 * auto_bisect
 * bisect_tool
 * bisecter

Does anyone have any preference? They're all acceptable to me.

[shatch, 2014-06-30 16:34:23]

On 2014/06/29 22:47:30, qyearsley wrote:
> On 2014/06/28 04:47:15, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   mac_chromium_rel on tryserver.chromium
> >
>
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
> 
> The tests are failing because the package name "bisect" conflicts with the
> standard module name "bisect" (https://docs.python.org/2/library/bisect.html).
> 
> I think it's still possible to call the module by using absolute imports
("from
> __future__ import absolute_import"), but it may be simpler to just come up
with
> a name that doesn't conflict.
> 
> Still possible:
>  * auto_bisect
>  * bisect_tool
>  * bisecter
> 
> Does anyone have any preference? They're all acceptable to me.

I like bisect_tool, but any of them is fine really.

[qyearsley, 2014-06-30 20:48:02]

On 2014/06/30 16:34:23, shatch wrote:
> On 2014/06/29 22:47:30, qyearsley wrote:
> > On 2014/06/28 04:47:15, I haz the power (commit-bot) wrote:
> > > Try jobs failed on following builders:
> > >   mac_chromium_rel on tryserver.chromium
> > >
> >
>
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
> > 
> > The tests are failing because the package name "bisect" conflicts with the
> > standard module name "bisect"
(https://docs.python.org/2/library/bisect.html).
> > 
> > I think it's still possible to call the module by using absolute imports
> ("from
> > __future__ import absolute_import"), but it may be simpler to just come up
> with
> > a name that doesn't conflict.
> > 
> > Still possible:
> >  * auto_bisect
> >  * bisect_tool
> >  * bisecter
> > 
> > Does anyone have any preference? They're all acceptable to me.
> 
> I like bisect_tool, but any of them is fine really.

 * tools/bisect_tool is slightly redundant, but that's OK.
 * tools/auto_bisect may not be not 100% accurate if it's used for manual bisect
but that's not a huge problem.
 * bisector with an "o" is also possible, and sounds more like the name of a
killer robot.

I'm slightly in favor of auto_bisect, I think. I'll change the name and submit
when I get feedback from Tony!

[shatch, 2014-06-30 21:27:58]

Believe Tony originally named the bisect bot "Robocop", so he may be on board
with the whole killer robot naming.

On 2014/06/30 20:48:02, qyearsley wrote:
> On 2014/06/30 16:34:23, shatch wrote:
> > On 2014/06/29 22:47:30, qyearsley wrote:
> > > On 2014/06/28 04:47:15, I haz the power (commit-bot) wrote:
> > > > Try jobs failed on following builders:
> > > >   mac_chromium_rel on tryserver.chromium
> > > >
> > >
> >
>
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
> > > 
> > > The tests are failing because the package name "bisect" conflicts with the
> > > standard module name "bisect"
> (https://docs.python.org/2/library/bisect.html).
> > > 
> > > I think it's still possible to call the module by using absolute imports
> > ("from
> > > __future__ import absolute_import"), but it may be simpler to just come up
> > with
> > > a name that doesn't conflict.
> > > 
> > > Still possible:
> > >  * auto_bisect
> > >  * bisect_tool
> > >  * bisecter
> > > 
> > > Does anyone have any preference? They're all acceptable to me.
> > 
> > I like bisect_tool, but any of them is fine really.
> 
>  * tools/bisect_tool is slightly redundant, but that's OK.
>  * tools/auto_bisect may not be not 100% accurate if it's used for manual
bisect
> but that's not a huge problem.
>  * bisector with an "o" is also possible, and sounds more like the name of a
> killer robot.
> 
> I'm slightly in favor of auto_bisect, I think. I'll change the name and submit
> when I get feedback from Tony!

[qyearsley, 2014-06-30 21:35:04]

On 2014/06/30 21:27:58, shatch wrote:
> Believe Tony originally named the bisect bot "Robocop", so he may be on board
> with the whole killer robot naming.

Random fact: The popularity of the search term "bisector" (along with other math
terms) shoots up every September (after summer vacation ends).

http://www.google.com/trends/explore#q=bisector%2C%20%22angle%20bisector%22%2...

[qyearsley, 2014-07-09 01:29:00]

Changed directory name to auto_bisect and started try jobs again.

Note: The name chosen here may not be such a big deal since we're hoping to
implement the perf bisect functionality as a recipe.

[qyearsley, 2014-07-09 22:21:37]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2014-07-09 22:22:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qyearsley@chromium.org/359013002/60001

[commit-bot: I haz the power, 2014-07-10 01:27:14]

Change committed as 282209