Splits bench_expectations into per-bot files in preparation for growth. (issue 24075008)

[ben...@google.com]

Reviewers: robertphillips,

Message:
Hi Rob, this is the first step of a series of changes to facilitate bench
rebaselining.

Description:
Splits bench_expectations into per-bot files in preparation for growth.
Next steps are switching bots to using per-bot expectations, and deleting
the
existing bench_expectations.txt.

Please review this at https://codereview.chromium.org/24075008/

SVN Base: http://skia.googlecode.com/svn/trunk/

Affected files (+2091, -0 lines):
A bench/bench_expectations_Perf-Android-Nexus7-Tegra3-Arm7-Release.txt
A
bench/bench_expectations_Perf-Ubuntu12-ShuttleA-ATI5770-x86-Release.txt
A bench/bench_expectations_Perf-Win7-ShuttleA-HD2000-x86-Release.txt

[ben...@google.com]

On 2013/09/17 17:05:33, borenet wrote:
> Drive-by comment: this would be much easier to read if you'd created the
> files
> by running "svn cp" from the single bench_expectations file to the files
> for
the
> individual bots and then removed the irrelevant lines. That way, we
> could be
> sure that none of the expectations changed in this CL.

Thanks Eric. I thought about it and didn't go that route. The reasons were:
- I deleted some expectations that we no longer run (say, config
tile_1024x64)
- I sorted the lines in a different way to facilitate batch copy/paste in
the
coming dashboard-based solution

If I did svn cp since the line orders have changed it'll still be really
hard to
see any missing ones. As for expectation values, I did read from the
existing
bench_expectations.txt to create the new ones so they should be the same.
Thanks
for the suggestion though.

https://codereview.chromium.org/24075008/

[ben...@google.com]

On 2013/09/17 17:12:35, borenet wrote:

> In that case, I'd suggest doing this in multiple parts, or at least add
comments
> wherever anything has changed, just so that it's really clear.

Right, sorry for the lack of comments and explanations. Will do in the
future.

https://codereview.chromium.org/24075008/

[robertp...@google.com]

lgtm but I think Eric should be happy too.

https://codereview.chromium.org/24075008/

[ben...@google.com]

Committed patchset #1 manually as r11323 (presubmit successful).

https://codereview.chromium.org/24075008/

[epo...@google.com]

On 2013/09/17 17:10:13, benchen wrote:
> On 2013/09/17 17:05:33, borenet wrote:
> > Drive-by comment: this would be much easier to read if you'd created the
files
> > by running "svn cp" from the single bench_expectations file to the
> files for
> the
> > individual bots and then removed the irrelevant lines. That way, we
> could
be
> > sure that none of the expectations changed in this CL.

> Thanks Eric. I thought about it and didn't go that route. The reasons
> were:
> - I deleted some expectations that we no longer run (say, config
> tile_1024x64)
> - I sorted the lines in a different way to facilitate batch copy/paste in
> the
> coming dashboard-based solution

One way to handle that (in the future) would be to purposefully stage the
CL in
several patchsets. Maybe something like this:

Patchset 1: use svn cp to copy the single bench_expectations file to all the
per-builder files, and remove the single file
Patchset 2: for each per-builder file, remove the lines that do not pertain
to
this builder
Patchset 3: delete expectations we no longer run
Patchset 4: change order of some lines

Then, reviewers can see the difference at each patchset and perform a
meaningful
review.

> If I did svn cp since the line orders have changed it'll still be really
> hard
to
> see any missing ones. As for expectation values, I did read from the
> existing
> bench_expectations.txt to create the new ones so they should be the same.
Thanks
> for the suggestion though.

https://codereview.chromium.org/24075008/

[ben...@google.com]

This sounds to be the safest and clearest. Wish I had this thought. Thanks.

https://codereview.chromium.org/24075008/