Bug 1480006 (android-lto)

Enable LTO on Android builds

RESOLVED FIXED in Firefox 66

Status

defect
RESOLVED FIXED
11 months ago
4 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Depends on 1 bug)

unspecified
mozilla66
Unspecified
Android
Dependency tree / graph

Firefox Tracking Flags

(geckoview64 wontfix, geckoview65 wontfix, geckoview66 fixed, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

Details

(Whiteboard: [geckoview:p2])

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

11 months ago
No description provided.
Assignee

Updated

10 months ago
No longer depends on: 1480005
Assignee

Comment 1

10 months ago
Note this depends on an update of NSS for bug 1478623.
Attachment #8999093 - Flags: review?(core-build-config-reviews)
Comment on attachment 8999093 [details] [diff] [review]
Enable LTO on Android CI builds

Review of attachment 8999093 [details] [diff] [review]:
-----------------------------------------------------------------

Let's do this.
Attachment #8999093 - Flags: review?(core-build-config-reviews) → review+

Comment 3

10 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/681777ca2f49
Enable LTO on Android CI builds. r=froydnj

Comment 4

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/681777ca2f49
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Comment 5

10 months ago
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/a62c083163d6
Enable LTO on Android CI builds a=bustage-fix
Depends on: 1483190
Assignee: mh+mozilla → hskupin
Comment on attachment 8999991 [details] [diff] [review]
Enable LTO on Android CI builds

Not sure what went wrong here with `hg bzexport`. Sorry.
Attachment #8999991 - Attachment is obsolete: true
Attachment #8999991 - Flags: review?(ato)
Assignee: hskupin → mh+mozilla
Depends on: 1485759
Assignee

Comment 8

10 months ago
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1507e086acbcf9e0c60cfa7e89dd31995950e520 for multiple reasons (see commit message).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The recent backouts of either this bug or bug 1483190 improved build times on these Android platforms:

== Change summary for alert #15367 (as of Tue, 28 Aug 2018 10:28:09 GMT) ==

Improvements:

 51%  build times android-5-0-aarch64 opt taskcluster-c4.2xlarge     1,545.18 -> 757.70
 47%  build times android-4-0-armv7-api16 opt taskcluster-m4.2xlarge 1,640.77 -> 871.70

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=15367

Comment 10

10 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9481b9ecaf2f
Enable LTO on Android CI builds. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/9481b9ecaf2f
Status: REOPENED → RESOLVED
Closed: 10 months ago10 months ago
Resolution: --- → FIXED
Target Milestone: mozilla63 → mozilla64
Assignee

Updated

9 months ago
Depends on: 1489105
Assignee

Comment 12

8 months ago
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/7a0bdd661bbc for bug 1489953.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
The backout brought back a small compiler related regression. It also improved the build times.

== Change summary for alert #16837 (as of Tue, 16 Oct 2018 03:19:42 GMT) ==

Regressions:

  4%  compiler_metrics num_static_constructors android-5-0-aarch64 opt      76.00 -> 79.00

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=16837
Duplicate of this bug: 909856
Alias: android-lto
Depends on: 1489953
OS: Unspecified → Android
Whiteboard: [geckoview:p2]

Comment 15

8 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/444ecc63baf3
Enable LTO on Android CI builds. r=froydnj

Comment 16

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/444ecc63baf3
Status: REOPENED → RESOLVED
Closed: 10 months ago8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65

Updated

8 months ago
Depends on: 1502006
Depends on: 1502126
Assignee

Updated

8 months ago
No longer depends on: 1502021
Backed out changeset 444ecc63baf3 (Bug 1480006) for crashes on Android (Bug 1489553). a=backout

Backout push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&revision=700e65c0ab9bb8ec1182b890bd51722b95eb0f24
Status: RESOLVED → REOPENED
Flags: needinfo?(mh+mozilla)
Resolution: FIXED → ---
Target Milestone: mozilla65 → ---
Backout from comment 12 reduced the build times:

== Change summary for alert #16889 (as of Tue, 16 Oct 2018 12:35:29 GMT) ==

Improvements:

 42%  build times android-4-2-x86 opt taskcluster-c4.2xlarge     1,680.24 -> 967.25

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=16889
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #19)
> Backout from comment 12 reduced the build times:
> 
> == Change summary for alert #16889 (as of Tue, 16 Oct 2018 12:35:29 GMT) ==
> 
> Improvements:
> 
>  42%  build times android-4-2-x86 opt taskcluster-c4.2xlarge     1,680.24 ->
> 967.25
> 
> For up to date results, see:
> https://treeherder.mozilla.org/perf.html#/alerts?id=16889

We noticed the same pattern for the backout in comment 18.
See Also: → 1503330

Comment 21

5 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ab46880d921
Enable LTO on Android CI builds. r=froydnj

Comment 22

5 months ago
bugherder
Status: REOPENED → RESOLVED
Closed: 8 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

(In reply to Pulsebot from comment #21)

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ab46880d921
Enable LTO on Android CI builds. r=froydnj

Thanks!

Note that ARM64 Fennec builds will be published for the first time in the Fennec Nightly channel (bug 1368484) ~tomorrow, so we might see some new crash signatures from ARM64 builds that are not caused by LTO.

Here is a query for just ARMv7 Fennec 66 Nightly crash reports submitted since today:

https://crash-stats.mozilla.com/search/?cpu_arch=arm&product=FennecAndroid&version=66.0a1&date=%3E%3D2019-01-14T03%3A18%3A00.000Z&date=%3C2019-01-31T11%3A18%3A00.000Z

Here is a query for just ARM64 Fennec 66 Nightly crash reports submitted since today:

https://crash-stats.mozilla.com/search/?cpu_arch=arm64&product=FennecAndroid&version=66.0a1&date=%3E%3D2019-01-14T03%3A18%3A00.000Z&date=%3C2019-01-31T11%3A18%3A00.000Z

Assignee

Comment 24

5 months ago

At least, nsTimerImpl::Release doesn't appear in there yet. Touch wood.

Flags: needinfo?(mh+mozilla)

Ionuț, I think Perfherder alerts #18712 and #18714 are caused by this bug. Enabling Android LTO improved ARMv7 Fennec's Speedometer score by 2.5% on both the Moto G5 and Pixel 2.

Unfortunately, LTO regressed AArch64 Fennec's Speedometer score by -4.86%. Note that AArch64 Fennec is only using SpiderMonkey's Baseline JIT. Performance should improve once the Ion JIT supports AArch64. OTOH, we saw a similar AArch64 regression after updating to clang 7 (bug 1503330) that was fixed by disabling clang's machine outliner (bug 1508547).

https://treeherder.mozilla.org/perf.html#/alerts?id=18712

https://treeherder.mozilla.org/perf.html#/alerts?id=18714

You can see Speedometer's performance boost in this Perfherder graph:

https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-inbound,1794835,1,10&series=mozilla-inbound,1794674,1,10&series=mozilla-inbound,1794497,1,10

Flags: needinfo?(igoldan)

Ionuț, is there a bug for the alert #18714 for the AArch64 regression above? If not, I can file one.

(In reply to Chris Peterson [:cpeterson] from comment #26)

Ionuț, is there a bug for the alert #18714 for the AArch64 regression above? If not, I can file one.

Thanks for checking this! I filed bug 1520140.

Flags: needinfo?(igoldan)

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #27)

(In reply to Chris Peterson [:cpeterson] from comment #26)

Ionuț, is there a bug for the alert #18714 for the AArch64 regression above? If not, I can file one.

Thanks for checking this! I filed bug 1520140.

Sorry, I wanted to say bug 1520149.

Depends on: 1530057
See Also: → 1526396
You need to log in before you can comment on or make changes to this bug.