1581062 - 0.21 - 0.25% installer size (android-5-0-aarch64, linux64, osx-shippable, windows2012-64) regression on push b23b78103bc0d1e74e06b9148d5e0999a6f266c3 (Fri September 13 2019)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Alexandru Ionescu (needinfo me) [:alexandrui]]

We have detected a build metrics regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=b23b78103bc0d1e74e06b9148d5e0999a6f266c3

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

0.25% installer size android-5-0-aarch64 opt 51,659,718.46 -> 51,789,456.00
0.22% installer size linux64 opt 68,644,272.46 -> 68,796,142.17
0.22% installer size windows2012-64 opt 70,646,301.21 -> 70,800,808.58
0.21% installer size osx-shippable opt nightly 80,594,275.12 -> 80,765,648.00

You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=23047

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Comment 1

[Alexandru Ionescu (needinfo me) [:alexandrui]]

As alert 23046 shows, this might come from part 11.
Also, maybe this comparison helps.

Comment 2

[Bastien Orivel [:eijebong]]

Part 11 was pushed after the updates to fix a test. It's just a number change in an assert. I'll have a look at this tonight

Comment 3

[Bastien Orivel [:eijebong]]

My guess would be that updating regex is the cause. They added some pretty big tables.
I'm unsure how to proceed regarding verifying that claim as I can't find anything related to rust in the installer.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Nathan, is this significant enough to warrant further investigation?

Comment 5

[Nathan Froyd [:froydnj]]

It looks like regex-syntax has the unicode feature on by default, which I really doubt anybody needs. Can you have a go at turning that off, Bastien?

Comment 6

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Any updates here, Bastien?

Comment 7

[Bastien Orivel [:eijebong]]

I've started looking at it. Unfortunately since cargo features are additive, this is a lot of work. Especially since bindgen requires the unicode feature and https://github.com/rust-lang/cargo/issues/5730#issuecomment-590385876 just landed. Until this is stable, I don't think there's anything we can really do.

I can submit a few patches here and there to disable the feature but I don't think it'll have any impact

Comment 8

[Bastien Orivel [:eijebong]]

Attachment: Bug 1581062 - Part 1: Don't use the heavyweight feature of Inflector. r?froydnj

binast only uses one function from Inflector and it doesn't require any
feature.

Comment 9

[Bastien Orivel [:eijebong]]

Attachment: Bug 1581062 - Part 2: Remove the unicode feature from a few crates. r?froydnj,emilio

Depends on D65862

Comment 10

[Bastien Orivel [:eijebong]]

https://github.com/sebasmagri/env_logger/pull/154
https://github.com/binast/binjs-ref/pull/458

With the patches I submitted and the fix in Cargo should be enough to disable the unicode feature outside of build deps.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/485c738acdb1
Part 1: Don't use the heavyweight feature of Inflector.  r=froydnj
https://hg.mozilla.org/integration/autoland/rev/c333f6f9d1bd
Part 2: Remove the unicode feature from a few crates.  r=froydnj,emilio,jgraham,webdriver-reviewers

Comment 12

[Dorel Luca [:dluca]]

Backed out 2 changesets (bug 1581062) for Toolchain failures. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=294745070&repo=autoland

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=c333f6f9d1bd5839658b6dd0b8459de9d24a6b9c

Backout:
https://hg.mozilla.org/integration/autoland/rev/df3cdb2a806fb023ecd28cde98b4ec0f5ccbafcf

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:eijebong, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1af8bf84162e
Part 1: Don't use the heavyweight feature of Inflector.  r=froydnj
https://hg.mozilla.org/integration/autoland/rev/723b31980210
Part 2: Remove the unicode feature from a few crates.  r=froydnj,emilio,jgraham,webdriver-reviewers

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/1af8bf84162e
https://hg.mozilla.org/mozilla-central/rev/723b31980210