1803207 - Investigate and catalog omni.ja contents in GeckoView

Status: NEW

(GeckoView :: General, task, P4)

Description

[Jonathan Almeida [:jonalmeida]]

During a performance investigation, Arturo, Christian and I noticed an uptick in our APK size by 1.7Mb. After looking into this, we discovered that this was a regression documented in bug 1799002 which was fixed that caused the apk increase that added some locales which were missing.

However, we noticed that there were locales which were packaged into the omni.ja which were also not required too because they are already localized in the application side (addresses and credit cards). For example, in chrome/en-GB/locale/en-GB/browser/browser.properties we have resources for webextPerms which are unused in GeckoView/Fenix.

These duplicates are also packaged in a way that we can't strip them out after the fact the way we can locale strings.xml and layout files.

This issue is therefore to investigate what are the things that live in the omni.ja for Android and see if there are better ways to avoid packaging resources.

Comment 1

[Jonathan Almeida [:jonalmeida]]

Attachment: locale_in_omnija.txt

Attached is the diff of files that were re-introduced after the regression fix that have locales we need and some that we don't need.

If we were to strip out only the chrome/ locales that are unneeded, that would save us 0.97Mb. Maybe there are more places we can save on space?

Comment 2

[Nick Alexander :nalexander [he/him]]

Two things:

1. can you give more complete details on what's happening? I.e., which APKs you are comparing? (A listing with wget ... and then unpacking would help.)
2. I really expect the extra size is "necessary". You're comparing an en-US APK against a multi-locale APK; you'll find that any string provided by Gecko itself won't be translated. Now, there are not many such Gecko strings exposed... but there are some. Better to figure out which are truly not reachable from GeckoView and stop packaging them than stop packaging a multi-locale build entirely, I think.

Comment 3

[Jonathan Almeida [:jonalmeida]]

1. We were looking at this perfherder graph: https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogDat[…]ix,4063376,1,2&series=fenix,4063381,1,2&timerange=1209600
2. Maybe our graph with multi-locale is not a good comparison for apk sizes? Do you think it's worth investigating other things in the omni.ja that we might not need?

Better to figure out which are truly not reachable from GeckoView and stop packaging them than stop packaging a multi-locale build entirely, I think.

Yeah, this is the approach we spoke about offline that we might take. It's hard to avoid this problem in the future.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Jonathan Almeida [:jonalmeida] from comment #3)

1. We were looking at this perfherder graph: https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogDat[…]ix,4063376,1,2&series=fenix,4063381,1,2&timerange=1209600

Look with a longer time range and you see the regression neutering multi-locale packaging more clearly (including a backout, I expect):

https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&series=fenix,4063381,1,2&timerange=7776000

2. Maybe our graph with multi-locale is not a good comparison for apk sizes? Do you think it's worth investigating other things in the omni.ja that we might not need?

Yes, this is fertile ground. In fact, it looks like we duplicate https://searchfox.org/mozilla-central/source/mobile/android/branding/nightly/locales/en-US/brand.ftl unchanged across all locales. If that's true, we could avoid that and fall-back to English easily enough, saving some kB.

Comment 5

[Jonathan Almeida [:jonalmeida]]

(In reply to Nick Alexander :nalexander [he/him] from comment #4)

Look with a longer time range and you see the regression neutering multi-locale packaging more clearly (including a backout, I expect):

https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&series=fenix,4063381,1,2&timerange=7776000

Yes, I might have done a poor job with explaining this comment 0 that the regression and it's fixes are the context behind this bug being filed to see if there are more things aside from unused ftl files that we can strip. :)

Comment 6

[Chris Peterson [:cpeterson]]

(In reply to Jonathan Almeida [:jonalmeida] from comment #3)

1. We were looking at this perfherder graph: https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogDat[…]ix,4063376,1,2&series=fenix,4063381,1,2&timerange=1209600

FYI: the Nov 25 regression in APK size from 88 MB to 90+ MB was bundling PDF.js in the APK (bug 1754499). PDF.js is Nightly only for now and the Beta and Release builds won't bundle PDF.js until it's ready to ride the trains.

Comment 7

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

:calixte, since you are the author of the regressor, bug 1754499, could you take a look?

For more information, please visit auto_nag documentation.

Comment 8

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

This bug has the keyword regression, so its type should be defect.

Comment 9

[Chris Peterson [:cpeterson]]

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #7)

:calixte, since you are the author of the regressor, bug 1754499, could you take a look?

Clearing the needinfo for Calixte because this bug is about (only) about PDF.js bug 1754499 and PDF.js' APK size increase is expected.

Comment 10

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

This bug has the keyword regression, so its type should be defect.

Comment 11

[Chris Peterson [:cpeterson]]

Not needed for 110