Update Mac sizes perf expectations, reduce tolerance to 1%

Update Mac sizes perf expectations, reduce tolerance to 1%

Mac Chromium.app sizes expectations were last updated after the switch
to 64-bit which had a large increase (r305043 and
http://crbug.com/435576).

Later, there was a large drop for Chromium.app in r314575 due to the PDF
plugin being combined into the ChromiumFramework. The perf expectations
for ChromiumFramework were updated, but not Chromium.app.

This has meant that large jumps are unlikely to affect the Mac sizes
tree-closers, since they are absorbed into the gains from r314575.
Instead, large jumps are getting picked up late because they are triaged
from perf graphs rather than the Chrome waterfall.

Chrome has been growing about 1MB per month on average. Set the
tolerance to 1% (rather than 5%) for something likely to detect the
recent large erroneous jumps.

After this, Chromium.app will close the tree once it gets to 159,128kB.
The prior threshold was 165,929kB. Chromium.app currently reports
157,589kB

Also update ChromiumFramework. It's not out of whack, but it makes sense
for it to share a common threshold tolerance with Chromium.app.

BUG=468637
Committed: https://crrev.com/577e62cdc6721ed1b7c2819e00d0c9b3ebc5b46a
Cr-Commit-Position: refs/heads/master@{#327179}

[tapted, 2015-04-27 07:45:03]

tapted@chromium.org changed reviewers:
+ thakis@chromium.org

[tapted, 2015-04-27 07:48:39]

tapted@chromium.org changed reviewers:
+ miletus@chromium.org

[tapted, 2015-04-27 07:49:40]

Hi Nico, do you think these tolerances make sense?

miletus (current perf sheriff) - please take a look too.

Thanks!

[miletus1, 2015-04-27 16:07:52]

miletus@google.com changed reviewers:
+ sullivan@chromium.org

[miletus1, 2015-04-27 16:11:15]

miletus@google.com changed reviewers:
+ miletus@google.com

[miletus1, 2015-04-27 16:11:16]

Annie, 

This patch makes me wonder how do we update size expectation
for improvement in general ? Do we have a way to generate remainder to
rebaseline the size expectation after big size improvement ?

[sullivan, 2015-04-27 16:57:11]

On 2015/04/27 16:11:16, miletus1 wrote:
> Annie, 
> 
> This patch makes me wonder how do we update size expectation
> for improvement in general ? Do we have a way to generate remainder to
> rebaseline the size expectation after big size improvement ?

We don't really have a great process in place. We eventually want to move the
alerts to the perf dashboard and only alert on jumps over a certain size, but we
haven't implemented that yet. (It would be a threshold-based alerting algorithm
instead of an anomaly-based one)

[Nico, 2015-04-27 17:18:59]

rs-lgtm

[tapted, 2015-04-27 23:18:39]

On 2015/04/27 16:57:11, sullivan wrote:
> On 2015/04/27 16:11:16, miletus1 wrote:
> > Annie, 
> > 
> > This patch makes me wonder how do we update size expectation
> > for improvement in general ? Do we have a way to generate remainder to
> > rebaseline the size expectation after big size improvement ?
> 
> We don't really have a great process in place. We eventually want to move the
> alerts to the perf dashboard and only alert on jumps over a certain size, but
we
> haven't implemented that yet. (It would be a threshold-based alerting
algorithm
> instead of an anomaly-based one)

Note the large tolerance (5%) probably contributed significantly in this case
too. That is, sizes may have closed the tree sooner if Chromium.app was updated
after the PDF plugin change, but some big unintentional regressions (e.g.
http://crbug.com/471609 - a 1.5MB regression) may still have slipped through.

A 5% tolerance would need a 8MB delta to get picked up, which takes some effort.
The perf sheriff guide suggests a 100kB regression is a good threshold to revert
a change first. 1% tolerance becomes some middle ground here to try to pick up
significant things like http://crbug.com/471609 which probably should have been
reverted.

So does that all lg to a perf-sheriff? (e.g. is `"tolerance": 0.01` in the
appropriate lines of perf_expectations.json the right way to do it?)

[miletus1, 2015-04-27 23:30:14]

On 2015/04/27 23:18:39, tapted wrote:
> On 2015/04/27 16:57:11, sullivan wrote:
> > On 2015/04/27 16:11:16, miletus1 wrote:
> > > Annie, 
> > > 
> > > This patch makes me wonder how do we update size expectation
> > > for improvement in general ? Do we have a way to generate remainder to
> > > rebaseline the size expectation after big size improvement ?
> > 
> > We don't really have a great process in place. We eventually want to move
the
> > alerts to the perf dashboard and only alert on jumps over a certain size,
but
> we
> > haven't implemented that yet. (It would be a threshold-based alerting
> algorithm
> > instead of an anomaly-based one)
> 
> Note the large tolerance (5%) probably contributed significantly in this case
> too. That is, sizes may have closed the tree sooner if Chromium.app was
updated
> after the PDF plugin change, but some big unintentional regressions (e.g.
> http://crbug.com/471609 - a 1.5MB regression) may still have slipped through.
> 
> A 5% tolerance would need a 8MB delta to get picked up, which takes some
effort.
> The perf sheriff guide suggests a 100kB regression is a good threshold to
revert
> a change first. 1% tolerance becomes some middle ground here to try to pick up
> significant things like http://crbug.com/471609 which probably should have
been
> reverted.
> 
> So does that all lg to a perf-sheriff? (e.g. is `"tolerance": 0.01` in the
> appropriate lines of perf_expectations.json the right way to do it?)

lgtm. It looks reasonable to me.

But as a perf-sheriff who is only on duty for 2 days a quarter, I really don't
have more insight into what a proper tolerance should be.

[tapted, 2015-04-27 23:38:04]

On 2015/04/27 23:30:14, miletus1 wrote:
> On 2015/04/27 23:18:39, tapted wrote:
> > 
> > So does that all lg to a perf-sheriff? (e.g. is `"tolerance": 0.01` in the
> > appropriate lines of perf_expectations.json the right way to do it?)
> 
> lgtm. It looks reasonable to me.
> 
> But as a perf-sheriff who is only on duty for 2 days a quarter, I really don't
> have more insight into what a proper tolerance should be.

let's try it out :). I'll be keeping an eye on things during the code purple,
and I've taken an interest in mac sizes. There's also http://crbug.com/479841
(hitting a 4GB image limit of dsymutil) which is making this a hot problem.

[tapted, 2015-04-27 23:38:25]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2015-04-27 23:39:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1101133004/1

[commit-bot: I haz the power, 2015-04-27 23:51:04]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-04-27 23:51:51]

Patchset 1 (id:??) landed as
https://crrev.com/577e62cdc6721ed1b7c2819e00d0c9b3ebc5b46a
Cr-Commit-Position: refs/heads/master@{#327179}

[Nico, 2015-05-27 05:20:13]

Did running make_expectations.py just work for you? For me it's dying since it's
getting git revisions it doesn't know how to handle:

$  tools/perf_expectations/make_expectations.py 
Traceback (most recent call last):
  File "tools/perf_expectations/make_expectations.py", line 380, in <module>
    sys.exit(Main(sys.argv))
  File "tools/perf_expectations/make_expectations.py", line 249, in Main
    if int(jsondata['rev']) <= revb:
ValueError: invalid literal for int() with base 10:
'1b62c66803aebebba3d7704bbbe2e9679c388131'

[tapted, 2015-05-27 05:23:31]

On 2015/05/27 05:20:13, Nico wrote:
> Did running make_expectations.py just work for you? For me it's dying since
it's
> getting git revisions it doesn't know how to handle:
> 
> $  tools/perf_expectations/make_expectations.py 
> Traceback (most recent call last):
>   File "tools/perf_expectations/make_expectations.py", line 380, in <module>
>     sys.exit(Main(sys.argv))
>   File "tools/perf_expectations/make_expectations.py", line 249, in Main
>     if int(jsondata['rev']) <= revb:
> ValueError: invalid literal for int() with base 10:
> '1b62c66803aebebba3d7704bbbe2e9679c388131'

Yeah make_expectations.py seemed to run normally when I did it on Mac for this.
Let me try it out again..

[tapted, 2015-05-27 05:35:05]

On 2015/05/27 05:23:31, tapted wrote:
> On 2015/05/27 05:20:13, Nico wrote:
> > Did running make_expectations.py just work for you? For me it's dying since
> it's
> > getting git revisions it doesn't know how to handle:
> > 
> > $  tools/perf_expectations/make_expectations.py 
> > Traceback (most recent call last):
> >   File "tools/perf_expectations/make_expectations.py", line 380, in <module>
> >     sys.exit(Main(sys.argv))
> >   File "tools/perf_expectations/make_expectations.py", line 249, in Main
> >     if int(jsondata['rev']) <= revb:
> > ValueError: invalid literal for int() with base 10:
> > '1b62c66803aebebba3d7704bbbe2e9679c388131'
> 
> Yeah make_expectations.py seemed to run normally when I did it on Mac for
this.
> Let me try it out again..

Yeah - no dice. Tried it again on Chromium.app as well and got the same failure.

$ git diff
diff --git a/tools/perf_expectations/perf_expectations.json
b/tools/perf_expectations/perf_expectations.json
index 6bfbeb8..6206ed5 100644
--- a/tools/perf_expectations/perf_expectations.json
+++ b/tools/perf_expectations/perf_expectations.json
@@ -1,7 +1,7 @@
 {"linux-release-64/sizes/chrome-bss/bss": {"reva": 311086, "revb": 311132,
"type": "absolute", "better": "lower", "improve": 453296, "regress": 501029,
"sha1": "d69fb86b"},
  "linux-release-64/sizes/chrome-data/data": {"reva": 315738, "revb": 315881,
"type": "absolute", "better": "lower", "improve": 4530816, "regress": 5008996,
"sha1": "d3b1596a"},
  "linux-release-64/sizes/chrome-si/initializers": {"reva": 281711, "revb":
281711, "type": "absolute", "better": "lower", "improve": 7, "regress": 7,
"tolerance": 0, "sha1": "b11fc43a"},
- "linux-release-64/sizes/chrome-text/text": {"reva": 319216, "revb": 319260,
"type": "absolute", "better": "lower", "improve": 100175313, "regress":
110735624, "sha1": "bacf8d86"},
+ "linux-release-64/sizes/chrome-text/text": {"reva": 331476, "revb": 331531,
"type": "absolute", "better": "lower", "improve": 100175313, "regress":
110735624, "sha1": "bacf8d86"},
  "linux-release-64/sizes/chrome/chrome": {"reva": 315738, "revb": 315881,
"type": "absolute", "better": "lower", "improve": 146067493, "regress":
161501324, "sha1": "599a4df5"},
  "linux-release-64/sizes/nacl_helper-bss/bss": {"reva": 282247, "revb": 282247,
"type": "absolute", "better": "lower", "improve": 257670, "regress": 284794,
"sha1": "4baf0f5e"},
  "linux-release-64/sizes/nacl_helper-data/data": {"reva": 315014, "revb":
315022, "type": "absolute", "better": "lower", "improve": 220172, "regress":
243348, "sha1": "7b14efc5"},

tapted@tapted-macpro2
/Volumes/bdd/tapted/git/4chromium/src/tools/perf_expectations
(20150527-Linux-Sizes)
$ ./make_expectations.py
Traceback (most recent call last):
  File "./make_expectations.py", line 380, in <module>
    sys.exit(Main(sys.argv))
  File "./make_expectations.py", line 249, in Main
    if int(jsondata['rev']) <= revb:
ValueError: invalid literal for int() with base 10:
'2aeb38cab96e4f1ff8677231ce55d3970c773005'

[Nico, 2015-05-27 05:36:18]

Filed https://code.google.com/p/chromium/issues/detail?id=492505

On Tue, May 26, 2015 at 10:35 PM, <tapted@chromium.org> wrote:

> On 2015/05/27 05:23:31, tapted wrote:
>
>> On 2015/05/27 05:20:13, Nico wrote:
>> > Did running make_expectations.py just work for you? For me it's dying
>> since
>> it's
>> > getting git revisions it doesn't know how to handle:
>> >
>> > $  tools/perf_expectations/make_expectations.py
>> > Traceback (most recent call last):
>> >   File "tools/perf_expectations/make_expectations.py", line 380, in
>> <module>
>> >     sys.exit(Main(sys.argv))
>> >   File "tools/perf_expectations/make_expectations.py", line 249, in Main
>> >     if int(jsondata['rev']) <= revb:
>> > ValueError: invalid literal for int() with base 10:
>> > '1b62c66803aebebba3d7704bbbe2e9679c388131'
>>
>
>  Yeah make_expectations.py seemed to run normally when I did it on Mac for
>>
> this.
>
>> Let me try it out again..
>>
>
> Yeah - no dice. Tried it again on Chromium.app as well and got the same
> failure.
>
> $ git diff
> diff --git a/tools/perf_expectations/perf_expectations.json
> b/tools/perf_expectations/perf_expectations.json
> index 6bfbeb8..6206ed5 100644
> --- a/tools/perf_expectations/perf_expectations.json
> +++ b/tools/perf_expectations/perf_expectations.json
> @@ -1,7 +1,7 @@
>  {"linux-release-64/sizes/chrome-bss/bss": {"reva": 311086, "revb": 311132,
> "type": "absolute", "better": "lower", "improve": 453296, "regress":
> 501029,
> "sha1": "d69fb86b"},
>   "linux-release-64/sizes/chrome-data/data": {"reva": 315738, "revb":
> 315881,
> "type": "absolute", "better": "lower", "improve": 4530816, "regress":
> 5008996,
> "sha1": "d3b1596a"},
>   "linux-release-64/sizes/chrome-si/initializers": {"reva": 281711, "revb":
> 281711, "type": "absolute", "better": "lower", "improve": 7, "regress": 7,
> "tolerance": 0, "sha1": "b11fc43a"},
> - "linux-release-64/sizes/chrome-text/text": {"reva": 319216, "revb":
> 319260,
> "type": "absolute", "better": "lower", "improve": 100175313, "regress":
> 110735624, "sha1": "bacf8d86"},
> + "linux-release-64/sizes/chrome-text/text": {"reva": 331476, "revb":
> 331531,
> "type": "absolute", "better": "lower", "improve": 100175313, "regress":
> 110735624, "sha1": "bacf8d86"},
>   "linux-release-64/sizes/chrome/chrome": {"reva": 315738, "revb": 315881,
> "type": "absolute", "better": "lower", "improve": 146067493, "regress":
> 161501324, "sha1": "599a4df5"},
>   "linux-release-64/sizes/nacl_helper-bss/bss": {"reva": 282247, "revb":
> 282247,
> "type": "absolute", "better": "lower", "improve": 257670, "regress":
> 284794,
> "sha1": "4baf0f5e"},
>   "linux-release-64/sizes/nacl_helper-data/data": {"reva": 315014, "revb":
> 315022, "type": "absolute", "better": "lower", "improve": 220172,
> "regress":
> 243348, "sha1": "7b14efc5"},
>
> tapted@tapted-macpro2
> /Volumes/bdd/tapted/git/4chromium/src/tools/perf_expectations
> (20150527-Linux-Sizes)
> $ ./make_expectations.py
> Traceback (most recent call last):
>   File "./make_expectations.py", line 380, in <module>
>     sys.exit(Main(sys.argv))
>   File "./make_expectations.py", line 249, in Main
>     if int(jsondata['rev']) <= revb:
> ValueError: invalid literal for int() with base 10:
> '2aeb38cab96e4f1ff8677231ce55d3970c773005'
>
>
>
> https://codereview.chromium.org/1101133004/
>

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