Update Linux size expectations to recent range of my code addition.

Update Linux size expectations to recent range of my code addition.

[brettw, 2011-12-11 20:00:05]

Bump more expectations. I spent quite a while and can't really figure out
exactly what to update, so I updated all Linux and Linux-64 ones.

[brettw, 2011-12-11 20:00:27]

TBR

[Nico, 2011-12-11 21:20:13]

http://codereview.chromium.org/8913001/diff/2001/tools/perf_expectations/perf...
File tools/perf_expectations/perf_expectations.json (right):

http://codereview.chromium.org/8913001/diff/2001/tools/perf_expectations/perf...
tools/perf_expectations/perf_expectations.json:1:
{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113953, "revb": 113962,
"type": "absolute", "improve": 595081, "regress": 657756, "sha1": "c1e34d47"},
That does nothing. You need to run `make_expectations" after updating revisions
for this to have an effect, see
http://www.chromium.org/developers/tree-sheriffs/perf-sheriffs#TOC-Updating-p...

You only need to update the linux-release line (look in the URL of
http://build.chromium.org/f/chromium/perf/linux-release/sizes/report.html?his...
, linked from the red box on the waterfall). Not sure which of the sizes
subpaths, updating all is probably fine.

I agree the instructions could be better :-/

[cmp, 2011-12-12 20:36:20]

There are presubmit checks that give a clear error if one uploads a change like
this.  Brett, did you force the upload?  You replied 'TBR' but I don't see a
'Committed' line, did you use gcl to land this?

The presubmit error would say that "Expectations has pending updates."  The
instructions at
http://www.chromium.org/developers/tree-sheriffs/perf-sheriffs#TOC-Updating-p...
are clear that make_expectations.py needs to be run after changing this file.

thakis wrote:
> I agree the instructions could be better :-/

No one can ever claim they have written perfect instructions, but how
specifically could you make these instructions better?  I encourage you to
propose or make changes that make things better for everyone rather than skip
the opportunity.

[brettw, 2011-12-12 20:39:13]

On Mon, Dec 12, 2011 at 12:36 PM,  <cmp@chromium.org> wrote:
> There are presubmit checks that give a clear error if one uploads a change
> like
> this.  Brett, did you force the upload?  You replied 'TBR' but I don't see a
> 'Committed' line, did you use gcl to land this?
>
> The presubmit error would say that "Expectations has pending updates."  The
> instructions at
>
http://www.chromium.org/developers/tree-sheriffs/perf-sheriffs#TOC-Updating-p...
> are clear that make_expectations.py needs to be run after changing this
> file.

I forced the checkin. The first time I did it I forgot to run the
script so I aborted. Then I ran it, apparently it failed for reasons I
don't understand. Then I didn't really have anything else I knew to do
other than force the checkin again.

Brett

[cmp, 2011-12-12 20:56:53]

I copied the first line's change into my local perf_expectations.json copy and
made it myself.  I ran make_expectations.py.  This is what I saw:


linux-release-64/sizes/chrome-bss/bss:
  regress (625314.0) is equal to improve (625314.0), and "better" is
unspecified, please fix by setting "better": "lower" or "better": "higher" in
this perf trace's expectation

I verified on the graph:
http://build.chromium.org/f/chromium/perf/linux-release-64/sizes/report.html?...

linux-release-64/sizes/chrome-bss is constant over the range 113953:113962.  The
script saw this and gave the correct message since there's no way to figure out
which way is considered a regression.  I modified the line to add "better":
"lower" so that it knew where to put the lower value and where to put the higher
value (between improve and regress).  I reran the script and it printed:


cmp@terakrom:/work/chrome/trunk/src/tools/perf_expectations$
./make_expectations.py

linux-release-64/sizes/chrome-bss/bss:
  traces: {u'bss': {'high': 625314.0, 'low': 625314.0}}
  before: {u'reva': 113953, u'revb': 113962, u'better': u'lower', u'regress':
657756, u'type': u'absolute', u'improve': 595081}
  after: {u'reva': 113953, u'revb': 113962, u'better': u'lower', u'regress':
656580, u'type': u'absolute', u'improve': 594048}

Writing expectations...  done

I verified the new line in the file:


-{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113953, "revb": 113962,
"type": "absolute", "better": "lower", "improve": 595081, "regress": 657756,
"sha1": "c1e34d47"},
+{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113953, "revb": 113962,
"type": "absolute", "better": "lower", "improve": 594048, "regress": 656580,
"sha1": "365e97bd"},

This is functionally correct, although the practical difference between the old
and new lines is insignificant.  To make the values tighter and more likely to
trigger a regression or improvement, we could set "tolerance": 0 or to some
value > 0 and < 0.05.

I didn't check the rest of the changes to see if they had similar issues, but I
imagine they did given the tight revision range, the lack of "better" values in
those lines, and the fact that the test expectations being updated were all
sizes on Linux (a reliable test).

...

After looking through the changes, I see in some places that reva is 113963 and
revb is 113962.  I remember handling this at some point, I'll check that case to
verify what happens.

[cmp, 2011-12-12 21:02:27]

The script works fine with a reva that's higher than revb.  Internally, it
juggles them so it ensures that reva is lower than revb.  Here's a line with
113963:113962:

+{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113963, "revb": 113962,
"type": "absolute", "better": "lower", "improve": 595081, "regress": 657756,
"sha1": "c1e34d47"},

Here's a line with 113962:113963:

+{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113962, "revb": 113963,
"type": "absolute", "better": "lower", "improve": 594048, "regress": 656580,
"sha1": "0f4cb1b8"},

Based on this I believe there's nothing obviously wrong with how
make_expectations.py handles these cases, on my system at least.  Brett, can you
give more details on how it handled these cases on your system?  Was it a no-op
which didn't touch perf_expectations.json at all and didn't give any error
messages?

[brettw, 2011-12-12 21:08:26]

On Mon, Dec 12, 2011 at 1:02 PM,  <cmp@chromium.org> wrote:
> The script works fine with a reva that's higher than revb.  Internally, it
> juggles them so it ensures that reva is lower than revb.  Here's a line with
> 113963:113962:
>
> +{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113963, "revb": 113962,
>
> "type": "absolute", "better": "lower", "improve": 595081, "regress": 657756,
> "sha1": "c1e34d47"},
>
> Here's a line with 113962:113963:
>
> +{"linux-release-64/sizes/chrome-bss/bss": {"reva": 113962, "revb": 113963,
>
> "type": "absolute", "better": "lower", "improve": 594048, "regress": 656580,
> "sha1": "0f4cb1b8"},
>
> Based on this I believe there's nothing obviously wrong with how
> make_expectations.py handles these cases, on my system at least.  Brett, can
> you
> give more details on how it handled these cases on your system?  Was it a
> no-op
> which didn't touch perf_expectations.json at all and didn't give any error
> messages?

I got that error message, but I didn't know what it meant or what to
do about it.

Brett