Add script to rebaseline benches from codereview trybot results

Add script to rebaseline benches from codereview trybot results

This script is designed to be used by the RecreateSKPs bot.  Eventually, the bot will:
1. Generate new SKPs
2. Upload the new SKPs to a subdirectory with an ID or generation number.
3. Change Skia to use the new SKPs:
    a. Create and upload a Skia CL which changes the "current SKP generation" file to point to the new SKPs
    b. Launch Perf trybots on that CL.
    c. Call this script every 5 minutes until it successfully creates new baselines for each of the launched Perf bots.
    d. Add the new baselines to the CL
    e. Upload a second patch set of the CL
    f. Check the CQ bit on the CL

BUG=skia:2225
Committed: http://code.google.com/p/skia/source/detail?r=14921

[borenet, 2014-05-23 22:24:47]

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
File tools/gen_bench_expectations_from_codereview.py (right):

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:93: try_build.build_number))
Maybe the error_on_unfinished should be pulled out of this script eventually, so
that the RecreateSKPs bot can use this logic:

while not trybots_are_done():
  time.sleep(300)
generate_new_baselines()

Instead of the logic that the current state of this script implies:

while True:
  try:
    generate_new_baselines()
    break
  except:
    if failed_due_to_unfinished_trybots:
      time.sleep(5)

[benchen, 2014-05-23 22:51:02]

lgtm with an optional suggestion. Thank you!

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
File tools/gen_bench_expectations_from_codereview.py (right):

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:68: match =
re.match('bench_(?P<revision>[0-9a-fA-F]{2,40})_data*', data_file)
The files that end with _data are microbenches only. If we exclude them we can
narrow the search range, like
'bench_(?P<revision>[0-9a-fA-F]{2,40})_data_skp*'

[epoger, 2014-05-27 15:29:56]

Seems pretty handy...

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
File tools/gen_bench_expectations_from_codereview.py (right):

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:8: """Generate new bench
expectations from results of trybots on a code review."""
Is this tool intended to be run by humans?

If so: I think we'll need to add more documentation or help text, including
information about how to make the gsutil work (need .boto file).  I think it's
fine to add that later.

If not: maybe it should go in the skia-buildbot repo instead?

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:93: try_build.build_number))
On 2014/05/23 22:24:47, borenet wrote:
> Maybe the error_on_unfinished should be pulled out of this script eventually,
so
> that the RecreateSKPs bot can use this logic:
> 
> while not trybots_are_done():
>   time.sleep(300)
> generate_new_baselines()
> 
> Instead of the logic that the current state of this script implies:
> 
> while True:
>   try:
>     generate_new_baselines()
>     break
>   except:
>     if failed_due_to_unfinished_trybots:
>       time.sleep(5)

I think either one is fine...

Maybe we should throw a special subclass of Exception here, to make it easier to
figure out that it failed because of unfinished trybots?

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:101: dest_dir =
os.path.join('.bench_data', builder)
please use a constant to hold '.bench_data'

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:119: output_file =
os.path.join('expectations', 'bench',
This makes assumptions about what directory this script was run within. 
Probably safer to use absolute paths based on __file__.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:123:
subprocess.check_call(['python', 'bench/gen_bench_expectations.py',
Same as above (about CWD), but also / vs os.path.join().

[borenet, 2014-05-27 19:36:13]

Uploaded patch set 3.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
File tools/gen_bench_expectations_from_codereview.py (right):

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:8: """Generate new bench
expectations from results of trybots on a code review."""
On 2014/05/27 15:29:56, epoger wrote:
> Is this tool intended to be run by humans?
> 
> If so: I think we'll need to add more documentation or help text, including
> information about how to make the gsutil work (need .boto file).  I think it's
> fine to add that later.
> 
> If not: maybe it should go in the skia-buildbot repo instead?

For now, the primary user will be the RecreateSKPs bot, but I'd imagine this
would be pretty useful for developers as well:
I create a change which I expect to change perf results.  I set off some trybots
and then run this script to collect their results and generate new expectations.
 I can then add them to the CL before committing.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:68: match =
re.match('bench_(?P<revision>[0-9a-fA-F]{2,40})_data*', data_file)
On 2014/05/23 22:51:02, benchen wrote:
> The files that end with _data are microbenches only. If we exclude them we can
> narrow the search range, like
> 'bench_(?P<revision>[0-9a-fA-F]{2,40})_data_skp*'

It doesn't really matter for this use case; all of the files should have the
same revision, so it doesn't hurt to include the microbench result file.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:93: try_build.build_number))
On 2014/05/27 15:29:56, epoger wrote:
> On 2014/05/23 22:24:47, borenet wrote:
> > Maybe the error_on_unfinished should be pulled out of this script
eventually,
> so
> > that the RecreateSKPs bot can use this logic:
> > 
> > while not trybots_are_done():
> >   time.sleep(300)
> > generate_new_baselines()
> > 
> > Instead of the logic that the current state of this script implies:
> > 
> > while True:
> >   try:
> >     generate_new_baselines()
> >     break
> >   except:
> >     if failed_due_to_unfinished_trybots:
> >       time.sleep(5)
> 
> I think either one is fine...
> 
> Maybe we should throw a special subclass of Exception here, to make it easier
to
> figure out that it failed because of unfinished trybots?

Done.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:101: dest_dir =
os.path.join('.bench_data', builder)
On 2014/05/27 15:29:56, epoger wrote:
> please use a constant to hold '.bench_data'

Done.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:119: output_file =
os.path.join('expectations', 'bench',
On 2014/05/27 15:29:56, epoger wrote:
> This makes assumptions about what directory this script was run within. 
> Probably safer to use absolute paths based on __file__.

Done.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:123:
subprocess.check_call(['python', 'bench/gen_bench_expectations.py',
On 2014/05/27 15:29:56, epoger wrote:
> Same as above (about CWD), but also / vs os.path.join().

Done.

[benchen, 2014-05-27 20:51:00]

Thanks for the explanations. LGTM.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
File tools/gen_bench_expectations_from_codereview.py (right):

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:8: """Generate new bench
expectations from results of trybots on a code review."""
Yes, that'll be cool.
On 2014/05/27 19:36:14, borenet wrote:
> On 2014/05/27 15:29:56, epoger wrote:
> > Is this tool intended to be run by humans?
> > 
> > If so: I think we'll need to add more documentation or help text, including
> > information about how to make the gsutil work (need .boto file).  I think
it's
> > fine to add that later.
> > 
> > If not: maybe it should go in the skia-buildbot repo instead?
> 
> For now, the primary user will be the RecreateSKPs bot, but I'd imagine this
> would be pretty useful for developers as well:
> I create a change which I expect to change perf results.  I set off some
trybots
> and then run this script to collect their results and generate new
expectations.
>  I can then add them to the CL before committing.

https://codereview.chromium.org/297893004/diff/1/tools/gen_bench_expectations...
tools/gen_bench_expectations_from_codereview.py:68: match =
re.match('bench_(?P<revision>[0-9a-fA-F]{2,40})_data*', data_file)
That's right, thanks.
On 2014/05/27 19:36:14, borenet wrote:
> On 2014/05/23 22:51:02, benchen wrote:
> > The files that end with _data are microbenches only. If we exclude them we
can
> > narrow the search range, like
> > 'bench_(?P<revision>[0-9a-fA-F]{2,40})_data_skp*'
> 
> It doesn't really matter for this use case; all of the files should have the
> same revision, so it doesn't hurt to include the microbench result file.

[epoger, 2014-05-27 22:56:43]

LGTM at patchset 3

[borenet, 2014-05-28 16:40:03]

The CQ bit was checked by borenet@google.com

[commit-bot: I haz the power, 2014-05-28 16:40:15]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/borenet@google.com/297893004/40001

[commit-bot: I haz the power, 2014-05-28 17:30:12]

Change committed as 14921