1527135 - Incorrect stacks in profiler for webrender on geckoview

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Jamie Nicol [:jnicol]]

Attachment: Fennec 2019-02-11 18.09 profile.sps.json.gz

I've been trying to profile webrender using a local build of the geckoview example android app. Attached is a profile I captured. In the Renderer thread, you can see a whole bunch of functions as direct children of MessagePumpDefault::Run(), when lots of these should actually be descendants of rust code elsewhere in the profile. Interestingly all these functions seem to be from libs other than libxul.so, such as libGLESv2_adreno.so.

Device is a Oneplus 6 running Android P, but was seeing the same issue on my Pixel 2.

Markus told me on irc this is a stackwalking issue. Is there any more information I can provide?

Comment 1

[Markus Stange [:mstange]]

Jed, this is the problem I mentioned to you in Orlando. I can provide two libESXGLESv2_adreno.so files, one from before an Android update on Nov 18, 2018, and one from after the update. The "before" version had working unwinding, the "after" version the profiler is not able to unwind anymore.

Would that be enough information for you to take a look?

Comment 2

[Markus Stange [:mstange]]

This is currently interfering with work to get WebRender running on Android, so it would be great if we could fix it.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comparing the two libraries, the unwind tables and program headers look reasonable for both of them, but while looking at the code that reads the file header, I noticed an important difference.

Before (output from readelf -h):

  OS/ABI:                            UNIX - System V

After:

  OS/ABI:                            UNIX - GNU

I think the EI_OSABI and EI_ABIVERSION checks can just be deleted; I don't think there's any value of those fields that would change the format of the unwind tables.

Comment 4

[Markus Stange [:mstange]]

Thanks!! Removing those checks indeed gives me working stackwalking for the GL driver!

Here are two profiles, focused on the glTexSubImage2D function on the compositor thread.
Before: https://perfht.ml/2Ecnmhp
After: https://perfht.ml/2EcnlKn

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1527135 - Remove unnecessary EI_OSABI and EI_ABIVERSION checks. r?jld

Comment 6

[Markus Stange [:mstange]]

Jamie, can you check whether this patch improves the situation for you?
(I still don't get working unwinding out of libc.so, but I think that's a separate problem and I will file a new bug for it.)

Comment 7

[Jamie Nicol [:jnicol]]

Yes, it definitely improves it, thanks Jed and Markus! There are still some functions which look in the wrong place, but the ones I'm interested in are correct.

Here's a profile with the patch applied: https://perf-html.io/public/52a09cfbe9194e76598932e431f0ada65a0240b8/calltree/?globalTrackOrder=0-1&thread=1&v=3

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2302287136a3
Remove unnecessary EI_OSABI and EI_ABIVERSION checks. r=jld

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/2302287136a3