1557570 - Complete implementation of Base Profiler on Android

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P3)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Most of the Gecko Profiler's Android code was removed or commented-out when porting it to mozglue.
It would be great to make Base Profiler work on Android. I have little experience with Android, so I cannot give more details here yet.

Comment 1

[Markus Stange [:mstange]]

I've been looking into this a bit. I haven't gotten to the stage of testing it yet, I'm still trying to make it build.

The biggest problem with getting it to build is shared-libraries-linux.cc which contains some code from toolkit/crashreporter so that it can compute a build ID. That code uses the ElfW(...) macro, which is defined in <link.h>, but <link.h> conflicts with ElfLoader.h, which is included into shared-libraries-linux.cc when using MOZ_LINKER.
So I think we need to split the "compute build ID" code out into a separate compilation unit that can include <link.h> without running into Linker.h and friends.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

A build ID doesn't sound so important, maybe it could be removed, or have a near-empty implementation on Android that returns an arbitrary number?

Note that if you run both Base+Gecko Profilers (e.g., with both MOZ_BASE_PROFILER_STARTUP=1 and MOZ_PROFILER_STARTUP=1), only the "threads" section of the Base Profiler capture is kept, so the whole shared library stuff is discarded anyway!

Comment 3

[Markus Stange [:mstange]]

Oh, true. However, we'll have to solve this eventually if we want to unify the two profilers. We could even do the SharedLibraries part first: tools/profiler could use SharedLibraries from mozglue/baseprofiler before the rest of the profiler moves over.

I'll experiment with just #ifdef'ing out the build ID code out tomorrow.

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1557570 - WIP: BaseProfiler support for Android

Comment 5

[Markus Stange [:mstange]]

I got something working. Notes to myself so that I don't forget:

• separate out patch to parse MOZ_BASE_PROFILER_STARTUP_INTERVAL
• add proper EnsureBaseProfilerStarted() function to APKOpen.cpp and document why it's needed (no proper "root" function on android, Java calls various exported functions to load libraries. Also mention that getenv() doesn't work in static initializers.)
• separate out patch to make BaseProfiler logging use __android_log_print
• file a bug about not using printf for error messages during envvar parsing because those don't show up on Android
• separate out patch to make shared-libraries-linux.cc compile
• write a patch to add ipcmessages feature to the base profiler feature list so that, after env var unification (bug 1586939) we can specify the ipcmessages feature without crashing the base profiler due to unknown feature string
• file a bug about incomplete stacks from libxul.so in the base profiler, for example there's no parent frame for Gecko_FallibleAssignCString

Profiles: https://perfht.ml/39a3dp9 , https://perfht.ml/2PyuJ7M
The GeckoView-example Android app spends about 130ms in the pre-libxul stage. Around 40ms of that is spent in CustomElf::Relocate. A lot of the time is spent in addresses that might be JITted Java code. One sample seems like it was in a static constructor (?), the function is symbolicated as GLOBAL__sub_I_Unified_cpp_protocol_http2.cpp. The inability to unwind from syscalls in libc (bug 1529108) is annoying as usual.

mach run --no-install --setenv MOZ_BASE_PROFILER_STARTUP=1 \
           --setenv MOZ_BASE_PROFILER_STARTUP_INTERVAL=2 \
           --setenv MOZ_BASE_PROFILER_STARTUP_FEATURES=threads,js,stackwalk,leaf,screenshots,java \
           --setenv MOZ_BASE_PROFILER_STARTUP_FILTERS="GeckoMain,Compositor,Renderer,IPDL Background" \
           --setenv MOZ_PROFILER_STARTUP=1 \
           --setenv MOZ_PROFILER_STARTUP_INTERVAL=2 \
           --setenv MOZ_PROFILER_STARTUP_FEATURES=threads,js,stackwalk,leaf,screenshots,ipcmessages,java \
           --setenv MOZ_PROFILER_STARTUP_FILTERS="GeckoMain,Compositor,Renderer,IPDL Background"

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1557570 - Make baseprofiler/core/shared-libraries-linux.cc compile on Android. r=glandium

Depends on D64996

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1557570 - Build the base profiler on Android. r=gerald

Depends on D64997

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1557570 - Call baseprofiler::profiler_init() from the first mozglue function that runs. r=glandium

As far as I can tell there is no single entry-point into C++ code on Android.
Instead, GeckoThread and GeckoLibLoader call various functions to load libraries one-by-one.
We want to capture all that library loading in the profiler, so we need to kick off the base profiler at the beginning of whichever function is called first.

Depends on D64998

Comment 9

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6dec78f838e6c6ebd792a5ab7db354259e650ed

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3d33877f9de
Make baseprofiler/core/shared-libraries-linux.cc compile on Android. r=glandium
https://hg.mozilla.org/integration/autoland/rev/fc259e08c46a
Build the base profiler on Android. r=gerald
https://hg.mozilla.org/integration/autoland/rev/94d6601695df
Call baseprofiler::profiler_init() from the first mozglue function that runs. r=gerald

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/e3d33877f9de
https://hg.mozilla.org/mozilla-central/rev/fc259e08c46a
https://hg.mozilla.org/mozilla-central/rev/94d6601695df

Comment 12

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #25435 (as of Thu, 19 Mar 2020 04:12:16 GMT) ==

Improvements:

7% ts_paint linux64-shippable opt e10s stylo 199.92 -> 186.50
7% ts_paint_webext linux64-shippable opt e10s stylo 199.33 -> 186.33
5% ts_paint_webext linux64-shippable-qr opt e10s stylo 279.88 -> 267.00
4% ts_paint windows7-32-shippable opt e10s stylo 320.21 -> 307.92
4% ts_paint windows7-32-shippable opt e10s stylo 319.75 -> 308.08

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

Comment 13

[Markus Stange [:mstange]]

(In reply to Alexandru Ionescu :alexandrui (needinfo me) from comment #12)
This is actually from bug 1619032, which landed in the same push. (Can you update the alert?)

Comment 14

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Yes, thanks!