Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Validate fenix is zipaligned #12527

Closed
mcomella opened this issue Jul 13, 2020 · 7 comments
Closed

Validate fenix is zipaligned #12527

mcomella opened this issue Jul 13, 2020 · 7 comments
Assignees
Labels
performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Jul 13, 2020

zipalign is an archive alignment tool that provides important optimization to Android application (APK) files. The purpose is to ensure that all uncompressed data starts with a particular alignment relative to the start of the file. Specifically, it causes all uncompressed data within the APK, such as images or raw files, to be aligned on 4-byte boundaries. This allows all portions to be accessed directly with mmap() even if they contain binary data with alignment restrictions. The benefit is a reduction in the amount of RAM consumed when running the application.

Since our native libs may be uncompressed, we may have non-trivial uncompressed data and this could be a decent performance optimization.

In addressing mozilla-mobile/firefox-echo-show#327, I discovered the autograph signing server does not zipalign apks before building them. A plausible error would be for the release builds to not get zipaligned before being sent to the signing server: we should verify that zipalign this is happening.

┆Issue is synchronized with this Jira Task

@mcomella mcomella added the performance Possible performance wins label Jul 13, 2020
@mcomella
Copy link
Contributor Author

It does not seem like it's happening because zipalign is not found in our repository: https://searchfox.org/mozilla-mobile/search?path=%5Efenix&q=zipalign

@mcomella
Copy link
Contributor Author

imo, I think the autograph server should probably be zipaligning before signing: the steps are too tightly coupled to separate them (e.g. this problem may also exist in FFTV).

@github-actions github-actions bot added the needs:triage Issue needs triage label Jul 13, 2020
@eliserichards eliserichards removed the needs:triage Issue needs triage label Jul 14, 2020
@data-sync-user data-sync-user changed the title Validate fenix is zipaligned FNX3-13545 ⁃ Validate fenix is zipaligned Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-13545 ⁃ Validate fenix is zipaligned FNX-13608 ⁃ Validate fenix is zipaligned Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-13608 ⁃ Validate fenix is zipaligned FNX2-15308 ⁃ Validate fenix is zipaligned Aug 11, 2020
@kbrosnan kbrosnan changed the title FNX2-15308 ⁃ Validate fenix is zipaligned Validate fenix is zipaligned Aug 29, 2020
@mcomella mcomella self-assigned this Jan 29, 2021
@mcomella
Copy link
Contributor Author

mcomella commented Jan 29, 2021

It does not seem like it's happening because zipalign is not found in our repository: https://searchfox.org/mozilla-mobile/search?path=%5Efenix&q=zipalign

Looking at autograph, there are signs that we may have reimplemented zipalign, rather than call the distributed binary: https://github.com/mozilla-services/autograph/blob/df8afe05c0c3d252a5cbca9c57ff38b71cc67a4e/signer/apk/jar.go#L231-L234


I discovered there is a command to verify zipalign on the app (via SO):

zipalign -c -v 4 'path-to-apk'

I tested this on the Nightly APK and verification was successful. However, I'm still not confident about what's happening. My theories are:

  • We are zipaligning in autograph through our own code (but why didn't this work for echo? Maybe I was zipaligning locally which conflicted with the autograph alignment?)
  • Google now zipaligns automatically during the android build process
  • We're coincidentally aligned but may not be in the future (e.g. I tested zipalign on basically empty Android apps in debug and release configs and they were also aligned despite me not running it)

For action items, I think we should:

  • Verify with the autograph team that they're zipaligning
  • Before we determine building and signing the APK was a success, we should run the zipalign verification command in the fenix repository

@mcomella
Copy link
Contributor Author

Set to waiting: waiting for responses from other teams.

@g-k
Copy link

g-k commented Jan 29, 2021

Autograph is not zipaligning APKs.

Autograph signs all apks with the apk2 signer which shells out to apksigner. You can check that the signed APK has an APK v2 signature to verify that the apk2 signer was used (the other signer doesn't implement the v2 signature format).

It'd be good to zipalign before signing and check zip alignment after signing (like the referenced echo show PR does) https://developer.android.com/studio/command-line/zipalign

@mcomella
Copy link
Contributor Author

Discussion in #releaseduty-mobile seems to indicate that we may not be zipaligning builds. There is code to do it but it may be bypassed when we sign via autograph. I don't have the expertise to implement the fix so I'll have to defer to the releng team. My recommendation would be to :

- zipalign
- sign via autograph
- verify zipalign (after signing)
```

So it may be that we're only zipaligned by coincidence at this point.

@mcomella
Copy link
Contributor Author

To make the action item of this work simpler to track for other parties, I filed #17703 to make the change. Closing this ticket ("validate its zipaligned", which it isn't hence a follow-up) as completed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance Possible performance wins
Projects
None yet
Development

No branches or pull requests

3 participants