719536 - The Gecko Profiler add-on needs to be able to symbolicate for Linux

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[(no longer active)]

This work needs some help from someone who knows JS and ideally knows a bit about implementing Jetpack based add-ons.  I can help with ideas about how to implement the Windows symbolication, and I believe Benoit can do the same about Linux.

Comment 1

[Benoit Girard (:BenWa)]

See https://github.com/bgirard/Gecko-Profiler-Addon/blob/master/lib/symbolicate.js for how we're currently doing it for jsctypes.

I think the problem with Linux is that we strip them.

Comment 2

[(dormant account)]

Linux supports external symbols(not as nicely as windows), but it's possible.

Comment 3

[Benoit Girard (:BenWa)]

We already have a solution for windows under another bug so morphing this to be linux only.

Comment 4

[Benoit Girard (:BenWa)]

Ted currently we don't have the symbols in the profiling nightly on linux. Is it possible to ship them in profiling enable builds? We don't need debug info or line information we just need symbols for internal functions.

Comment 5

[Benoit Girard (:BenWa)]

We discussed this on IRC, here's a quick cut and paste of relevant bits:
ted2: our build system has a STRIP_FLAGS variable it uses while stripping
ted2: it looks like it's a little...broken though, since you can't currently specify it in a mozconfig
glandium: there are options to strip to selectively delete things.
glandium: BenWa: i think strip -d will do what you want
glandium: BenWa: if STRIP_FLAGS doesn't work, setting STRIP="strip -d" may work
ted2: we should fix STRIP_FLAGS
ted2: (you have to make package STRIP_FLAGS=...)
BenWa: If STRIP_FLAGS doesn't work I'll submit a patch in that bug 
ted2: it just needs 1) an AC_SUBST(STRIP_FLAGS) in configure.in, and b) a STRIP_FLAGS line in config/autoconf.mk.in

Comment 6

[Benoit Girard (:BenWa)]

Alright so doing STRIP_FLAGS="--strip-debug" on linux does the following: keeps our 250k symbols (nm libxul.so | wc -l). Increases the .gz from 22MB to 25MB, increase the decompressed libxul.so from 49MB to 69MB.

So we want to do it at a minimum in nightly-profiling. Do we want to do it under any other settings? All other nightlies?

Comment 7

[(not currently active) Ted Mielczarek]

Given that it's such a tiny change in package size, I have no opposition to turning it on in all nightly builds. Would it be useful for anything other than the built-in profiler? We have unstripped Mac nightlies because they're useful for Shark and the built-in sampler in Activity Monitor.

Comment 8

[Benoit Girard (:BenWa)]

It would have some (limited) value for debugging. GCC can get the top frame and perhaps a few more (but likely not much more) without frame pointers/unwinds.

Comment 9

[Benoit Girard (:BenWa)]

Attachment: Set STRIP_FLAGS on linux 32/64 nightly

Is this correct? I do not need AC_SUBST since the variable comes from the mozconfig and not the configure script correct?

I added the diff away from the bottom to not cause conflict with the automated mozilla-central->profiling merge.

Comment 10

[Benoit Girard (:BenWa)]

Just a heads up Ehsan about this patch. I don't believe it will cause conflicts but just in case.

Comment 11

[(not currently active) Ted Mielczarek]

Comment on attachment 612870 [details] [diff] [review]
Set STRIP_FLAGS on linux 32/64 nightly

Review of attachment 612870 [details] [diff] [review]:
-----------------------------------------------------------------

No, you do need an AC_SUBST, and a line in autoconf.mk.in to propogate the value of the variable.

Comment 12

[Benoit Girard (:BenWa)]

Attachment: Set STRIP_FLAGS on linux 32/64 nightly v2

I hope that goods. I'm surprised these changes are needed since I was able to export STRIP_FLAGS in my environment and have it work.

Comment 13

[(not currently active) Ted Mielczarek]

Yes, that would work fine locally, but setting it in the mozconfig doesn't have the equivalent effect on tinderbox.

Comment 14

[(not currently active) Ted Mielczarek]

Comment on attachment 612892 [details] [diff] [review]
Set STRIP_FLAGS on linux 32/64 nightly v2

Review of attachment 612892 [details] [diff] [review]:
-----------------------------------------------------------------

That should work fine.

Comment 15

[Benoit Girard (:BenWa)]

Waiting on tree closure.

Comment 16

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fabd3f2bc1e

Comment 17

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/7fabd3f2bc1e

Comment 18

[Benoit Girard (:BenWa)]

We're correctly shipping symbols in nightlies now, but the add-on doesn't use the correctly. Re-opening this bug to track add-on changes needed.

Comment 19

[Benoit Girard (:BenWa)]

Made corrections to the profiling add-on. Symbolication is now working correctly. Last issues that needs to be solved is bug 744667.

Comment 20

[Benoit Girard (:BenWa)]

Attachment: Expose library offsets

Comment 21

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a93b99783b8c

Comment 22

[Benoit Girard (:BenWa)]

The problem was that we were walking the stack correctly but we weren't handling offset properly in proc/maps segments. For some reason this happens in official nightly builds but not more local builds. Regardless this is strictly more correct.

Comment 23

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/a93b99783b8c