Make shippable fat AAR builds faster
Categories
(Firefox Build System :: Android Studio and Gradle Integration, enhancement)
Tracking
(firefox152 fixed)
| Tracking | Status | |
|---|---|---|
| firefox152 | --- | fixed |
People
(Reporter: RyanVM, Assigned: ahochheiden)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Right now, regular opt fat AAR builds run in under 10 minutes because they simply take the already-existing architecture-specific build artifacts and merge them into a single build.
However, shippable fat AAR builds require a new build to be created by the build since multi-l10n support is needed and the individual shippable artifacts don't support that. As a result, these jobs typically take 50+ minutes to run and end up being a bottleneck in the overall Android build pipeline as they block downstream AC/Fenix/Focus builds.
I've got a patch in bug 1918644 which cuts down on some of that runtime by switching to beefier instance types, but it seems like we'd ideally be able to get that multi-l10n work out of the fat AAR job entirely so that the shippable job can have a comparable runtime to the non-shippable job.
I understand that we probably don't want to default the arm32 shippable build to multi-l10n by default so things would Just Work, but I'm wondering if one option might be to add a new multi-l10n shippable build type that only the fat AAR depends on so it can be built in parallel with the other GeckoView builds.
Thoughts?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #0)
Right now, regular opt fat AAR builds run in under 10 minutes because they simply take the already-existing architecture-specific build artifacts and merge them into a single build.
OK, I see this.
However, shippable fat AAR builds require a new build to be created by the build since multi-l10n support is needed and the individual shippable artifacts don't support that. As a result, these jobs typically take 50+ minutes to run and end up being a bottleneck in the overall Android build pipeline as they block downstream AC/Fenix/Focus builds.
I don't think this is correct. I think that what is different is that the opt versions use an artifact build when bundling everything together, where-as the shippable variants do not. See https://searchfox.org/mozilla-central/rev/ab8043ccefe85c30ef78b97193918934d6aa93d1/taskcluster/kinds/build-fat-aar/kind.yml#192-196.
The reason for this is that the full build will deliver an AppConstants.sys.mjs and other small things that differ for artifact builds just like a regular build -- but with more architectures supported (and the multi-locale packaging step). We could probably do a little labour to arrange for the fat AAR builds to be an artifact build, but appear like a full build, and drive the fat AAR time down.
I've got a patch in bug 1918644 which cuts down on some of that runtime by switching to beefier instance types, but it seems like we'd ideally be able to get that multi-l10n work out of the fat AAR job entirely so that the shippable job can have a comparable runtime to the non-shippable job.
The multi-l10n bit is not expensive at all -- at least, not beyond cloning the l10n repos, which used to be slow and error prone but should now be quick and reliable post l10n-monorepo work.
I understand that we probably don't want to default the arm32 shippable build to multi-l10n by default so things would Just Work, but I'm wondering if one option might be to add a new multi-l10n shippable build type that only the fat AAR depends on so it can be built in parallel with the other GeckoView builds.
The multi-l10n piece is not, I believe, the bottleneck here, but: I also don't see why we don't want to have shippable builds be multi-l10n. It's just a little repackaging after the build is complete.
(In reply to Nick Alexander :nalexander [he/him] from comment #1)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #0)
Right now, regular opt fat AAR builds run in under 10 minutes because they simply take the already-existing architecture-specific build artifacts and merge them into a single build.
OK, I see this.
However, shippable fat AAR builds require a new build to be created by the build since multi-l10n support is needed and the individual shippable artifacts don't support that. As a result, these jobs typically take 50+ minutes to run and end up being a bottleneck in the overall Android build pipeline as they block downstream AC/Fenix/Focus builds.
I don't think this is correct. I think that what is different is that the opt versions use an artifact build when bundling everything together, where-as the shippable variants do not. See https://searchfox.org/mozilla-central/rev/ab8043ccefe85c30ef78b97193918934d6aa93d1/taskcluster/kinds/build-fat-aar/kind.yml#192-196.
The reason for this is that the full build will deliver an
AppConstants.sys.mjsand other small things that differ for artifact builds just like a regular build -- but with more architectures supported (and the multi-locale packaging step). We could probably do a little labour to arrange for the fat AAR builds to be an artifact build, but appear like a full build, and drive the fat AAR time down.
RyanVM: if you have cycles to improve this, could you try copy-pasting that block into the shippable build and then comparing the resulting APKs, possibly with a diffoscope task? I expect you'll find some small changes: AppConstants.sys.mjs, maybe buildconfig.html, maybe some FOG/JOG/Glean things.
|
Reporter | |
Comment 3•1 year ago
|
||
OK, I was misunderstanding this log then:
https://treeherder.mozilla.org/logviewer?job_id=474065094&repo=try&lineNumber=5717
But yes, at the end of the day, whether it's the multi-l10n bits or something else needed to do it, my hope is exactly this:
We could probably do a little labour to arrange for the fat AAR builds to be an artifact build, but appear like a full build, and drive the fat AAR time down.
|
|
Reporter | |
Comment 4•1 year ago
|
||
We mid-aired there a bit, but ^ is basically exactly that suggested change:
https://hg.mozilla.org/try/rev/8ad657ceeb0d4dc1faff3a222963e4b681edc210
Oh, I see -- out of the box, the multi-l10n step doesn't work for artifact builds, and that's why we see
[task 2024-09-12T18:15:20.161Z] 18:15:20 INFO - 18:15:20 INFO - Artifact builds do not support localization. If you know what you are doing, you can use:
In this case, we do know what we are doing, and we can probably do what I suggest in that error message :) That would address the multi-l10n pieces (but not any other bits like AppConstants.sys.mjs). I'm going to leave the NI to myself to revisit this.
|
||
Comment 6•1 year ago
|
||
These jobs also appear to generate crashreporter-symbols artifacts that are completely useless, as far as I can tell. The libxul.so generated from the fat AAR build appears to be discarded. The final AAR contains the binaries from the dependent tasks.
It's good that the binaries from the fat AAR build job aren't used, because they're not built with PGO and would have worse performance than the binaries from the actual shippable single-arch tasks.
But this means that we're currently storing 1200MB for no reason. Or even 2400MB if we're storing identical files separately:
220MB public/build/target.crashreporter-symbols.zip
1GB public/cidata/target.crashreporter-symbols-full.tar.zst
220MB public/build/en-US/target.crashreporter-symbols.zip
1GB public/build/en-US/target.crashreporter-symbols-full.tar.zst
(In reply to Markus Stange [:mstange] from comment #6)
These jobs also appear to generate crashreporter-symbols artifacts that are completely useless, as far as I can tell. The libxul.so generated from the fat AAR build appears to be discarded. The final AAR contains the binaries from the dependent tasks.
It's good that the binaries from the fat AAR build job aren't used, because they're not built with PGO and would have worse performance than the binaries from the actual shippable single-arch tasks.
But this means that we're currently storing 1200MB for no reason. Or even 2400MB if we're storing identical files separately:
220MB public/build/target.crashreporter-symbols.zip 1GB public/cidata/target.crashreporter-symbols-full.tar.zst 220MB public/build/en-US/target.crashreporter-symbols.zip 1GB public/build/en-US/target.crashreporter-symbols-full.tar.zst
Mmm, interesting.
|
|
||
Comment 8•1 year ago
|
||
Florian asked me for a profile. Here it is: https://share.firefox.dev/4ozj33R (from this job)
(In reply to Markus Stange [:mstange] from comment #8)
Florian asked me for a profile. Here it is: https://share.firefox.dev/4ozj33R (from this job)
It's not wrong to profile this, but it's also not particularly relevant; the issue here is structural and relatively well understood. We're simply doing too much (full build!) when we need some artifact-y mode that also supports l10n; that's possible but requires some work.
:jcristau recently made this much more pleasant to work on via Bug 1975355 so I expect to be able to do (or direct) that work in the next few weeks. We'll see!
|
Assignee | |
Comment 10•4 months ago
|
||
|
||
Updated•4 months ago
|
|
|
||
Updated•4 months ago
|
|
|
||
Updated•4 months ago
|
|
||
Comment 11•3 months ago
|
||
|
||
Comment 12•3 months ago
|
||
Backed out for causing Android multi-arch fat AAR Shippable bustages
|
|
||
Comment 13•3 months ago
|
||
|
|
Assignee | |
Updated•3 months ago
|
|
|
||
Comment 14•3 months ago
|
||
|
||
Comment 15•3 months ago
|
||
| bugherder | ||
Description
•