1480834 - Build GeckoView for x86_64

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P3)

Description

[Chris Peterson [:cpeterson]]

The Google Play Store will require all apps that include native code to include 64-bit builds (ARM64 and x86_64) by August 2019:

https://android-developers.googleblog.com/2017/12/improving-app-security-and-performance.html

TBD: Once we have x86_64 builds, should we test x86_64 instead of or in addition to x86? Do we have any telemetry showing how common x86_64 devices are compared to x86?

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Looks like the SOCs have supported x86_64 since the beginning  ~2008. 

https://en.wikichip.org/wiki/intel/microarchitectures/bonnell
https://en.wikichip.org/wiki/intel/microarchitectures/saltwell
https://en.wikichip.org/wiki/intel/microarchitectures/silvermont
https://en.wikichip.org/wiki/intel/microarchitectures/airmont

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 1. Add WebRTC config for x86_64 on Android; r=dminor

Add GN config files and updated moz.build files for building WebRTC on
Android x86_64.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 2. Add x86-64 build support; r=nalexander

Add x86-64 as an option for Android builds in the build system.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 3. Fix linker compile errors under x86-64; r=glandium

Add necessary checks to make the linker build on x86-64.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 4. Specify -Werror for pthread_cond_timedwait_monotonic_np check; r=glandium

On 64-bit Android, `pthread_cond_timedwait_monotonic_np` is only
available for API 28+. The issue is, even though the NDK header does not
define `pthread_cond_timedwait_monotonic_np` for API 27 and below, the
NDK library _does_ include the function (in order to support builds
targeting API 28+).

During our configure test, the compiler only emits a warning about
`pthread_cond_timedwait_monotonic_np` being undefined, but linking
succeeds because the function is present in the library. Because linking
succeeds, the test inadvertently passes. This patch adds a '-Werror'
flag to the compilation, so the warning turns into an error to make the
test fail.

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 5. Keep target flag when building for Android on Linux; r=glandium

Currently, when building for Android x86-64 on Linux x86-64, we drop the
'--target' flag, which causes the build to fail. This patch adds a check
for OS mismatch, so we keep the '--target' flag in this situation.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 9004665 [details]
Bug 1480834 - 4. Specify -Werror for pthread_cond_timedwait_monotonic_np check; r=glandium

Mike Hommey [:glandium] has approved the revision.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 9004666 [details]
Bug 1480834 - 5. Keep target flag when building for Android on Linux; r=glandium

Mike Hommey [:glandium] has approved the revision.

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 9004664 [details]
Bug 1480834 - 3. Fix linker compile errors under x86-64; r=glandium

Mike Hommey [:glandium] has approved the revision.

Comment 10

[Dan Minor [:dminor]]

Comment on attachment 9004662 [details]
Bug 1480834 - 1. Add WebRTC config for x86_64 on Android; r=dminor

Dan Minor [:dminor] has approved the revision.

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9004663 [details]
Bug 1480834 - 2. Add x86-64 build support; r=nalexander

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 0. Drop mips and update NDK platform to 16; r=jchen

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 6. Add Android x86-64 configs; r=nalexander

Add config files for building Android for x86-64.

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 7. Add Android x86-64 tasks; r=nalexander r=dustin r=jlorenzo

Add tasks for building Android for x86-64.

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1480834 - 8. Add x86-64 as rust target; r=nalexander r=froydnj

Add x86-64 as a rust target so we can build Android for x86-64.

Comment 16

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9008223 [details]
Bug 1480834 - 6. Add Android x86-64 configs; r=nalexander

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 17

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9008224 [details]
Bug 1480834 - 7. Add Android x86-64 tasks; r=nalexander r=dustin r=jlorenzo

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 18

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9008225 [details]
Bug 1480834 - 8. Add x86-64 as rust target; r=nalexander r=froydnj

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 19

[Nathan Froyd [:froydnj]]

Comment on attachment 9008225 [details]
Bug 1480834 - 8. Add x86-64 as rust target; r=nalexander r=froydnj

Nathan Froyd [:froydnj] has approved the revision.

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 9008224 [details]
Bug 1480834 - 7. Add Android x86-64 tasks; r=nalexander r=dustin r=jlorenzo

Dustin J. Mitchell [:dustin] pronoun: he has approved the revision.

Comment 21

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 9008224 [details]
Bug 1480834 - 7. Add Android x86-64 tasks; r=nalexander r=dustin r=jlorenzo

Johan Lorenzo [:jlorenzo] has approved the revision.

Comment 22

[Chris Peterson [:cpeterson]]

Once we have x86-64 builds of GV, James recommends that we test x86-64 instead of x86 (or run only smoke tests on x86).

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2eb3342c3ea4
0. Drop mips and update NDK platform to 16; r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d0a7ce39cfc
1. Add WebRTC config for x86_64 on Android; r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/b78b12273560
2. Add x86-64 build support; r=nalexander
https://hg.mozilla.org/integration/mozilla-inbound/rev/c12699660620
3. Fix linker compile errors under x86-64; r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/ced591ff6815
4. Specify -Werror for pthread_cond_timedwait_monotonic_np check; r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/704af02a849d
5. Keep target flag when building for Android on Linux; r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbceefcad976
6. Add Android x86-64 configs; r=nalexander
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dd3bc1e9fd7
7. Add Android x86-64 tasks; r=nalexander r=dustin r=jlorenzo
https://hg.mozilla.org/integration/mozilla-inbound/rev/3eff52855cdb
8. Add x86-64 as rust target; r=nalexander r=froydnj

Comment 24

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/2eb3342c3ea4
https://hg.mozilla.org/mozilla-central/rev/2d0a7ce39cfc
https://hg.mozilla.org/mozilla-central/rev/b78b12273560
https://hg.mozilla.org/mozilla-central/rev/c12699660620
https://hg.mozilla.org/mozilla-central/rev/ced591ff6815
https://hg.mozilla.org/mozilla-central/rev/704af02a849d
https://hg.mozilla.org/mozilla-central/rev/bbceefcad976
https://hg.mozilla.org/mozilla-central/rev/1dd3bc1e9fd7
https://hg.mozilla.org/mozilla-central/rev/3eff52855cdb

Comment 25

[Chris Peterson [:cpeterson]]

(In reply to Pulsebot from comment #23)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/1dd3bc1e9fd7
> 7. Add Android x86-64 tasks; r=nalexander r=dustin r=jlorenzo

Johan, do you know why this new Android x86-64 build task is listed on Treeherder as "android-5-0-x86_64 opt" even though android.yml specifies `description: "Android 5.0 x86-64 Opt"`?

https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/taskcluster/ci/build/android.yml#582,589

Comment 26

[Johan Lorenzo [:jlorenzo]]

I see the confusion! The description you pointed out is used by Taskcluster itself. See description field here: https://tools.taskcluster.net/groups/DJ_YBK4uQTq3HD2NdQlyVQ/tasks/I32BJ4JqTYW7BOkLLx_eAA/details.

Treeherder uses its own naming. You have a to make a PR like these ones: https://github.com/mozilla/treeherder/pull/2701 and https://github.com/mozilla/treeherder/pull/2737.