Last Comment Bug 719536 - The Gecko Profiler add-on needs to be able to symbolicate for Linux
: The Gecko Profiler add-on needs to be able to symbolicate for Linux
Product: Core
Classification: Components
Component: Gecko Profiler (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Benoit Girard (:BenWa)
Depends on:
Blocks: 713227
  Show dependency treegraph
Reported: 2012-01-19 12:00 PST by :Ehsan Akhgari
Modified: 2012-04-14 06:41 PDT (History)
13 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Set STRIP_FLAGS on linux 32/64 nightly (1.74 KB, patch)
2012-04-06 06:36 PDT, Benoit Girard (:BenWa)
ted: review-
Details | Diff | Splinter Review
Set STRIP_FLAGS on linux 32/64 nightly v2 (2.90 KB, patch)
2012-04-06 08:21 PDT, Benoit Girard (:BenWa)
ted: review+
Details | Diff | Splinter Review
Expose library offsets (1.37 KB, patch)
2012-04-12 18:06 PDT, Benoit Girard (:BenWa)
jmuizelaar: review+
Details | Diff | Splinter Review

Description :Ehsan Akhgari 2012-01-19 12:00:53 PST
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) 2012-01-19 12:04:05 PST
See for how we're currently doing it for jsctypes.

I think the problem with Linux is that we strip them.
Comment 2 (dormant account) 2012-01-19 14:09:00 PST
Linux supports external symbols(not as nicely as windows), but it's possible.
Comment 3 Benoit Girard (:BenWa) 2012-03-10 17:34:40 PST
We already have a solution for windows under another bug so morphing this to be linux only.
Comment 4 Benoit Girard (:BenWa) 2012-04-04 13:55:44 PDT
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) 2012-04-05 10:32:12 PDT
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, and b) a STRIP_FLAGS line in config/
Comment 6 Benoit Girard (:BenWa) 2012-04-05 14:42:35 PDT
Alright so doing STRIP_FLAGS="--strip-debug" on linux does the following: keeps our 250k symbols (nm | wc -l). Increases the .gz from 22MB to 25MB, increase the decompressed 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 Ted Mielczarek [:ted.mielczarek] 2012-04-06 05:22:53 PDT
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) 2012-04-06 06:32:37 PDT
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) 2012-04-06 06:36:16 PDT
Created attachment 612870 [details] [diff] [review]
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) 2012-04-06 06:37:06 PDT
Just a heads up Ehsan about this patch. I don't believe it will cause conflicts but just in case.
Comment 11 Ted Mielczarek [:ted.mielczarek] 2012-04-06 07:43:57 PDT
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 to propogate the value of the variable.
Comment 12 Benoit Girard (:BenWa) 2012-04-06 08:21:59 PDT
Created attachment 612892 [details] [diff] [review]
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 Ted Mielczarek [:ted.mielczarek] 2012-04-06 08:27:44 PDT
Yes, that would work fine locally, but setting it in the mozconfig doesn't have the equivalent effect on tinderbox.
Comment 14 Ted Mielczarek [:ted.mielczarek] 2012-04-06 08:29:18 PDT
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) 2012-04-06 10:09:48 PDT
Waiting on tree closure.
Comment 17 Matt Brubeck (:mbrubeck) 2012-04-09 10:10:53 PDT
Comment 18 Benoit Girard (:BenWa) 2012-04-11 07:17:02 PDT
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) 2012-04-11 18:00:55 PDT
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) 2012-04-12 18:06:23 PDT
Created attachment 614641 [details] [diff] [review]
Expose library offsets
Comment 22 Benoit Girard (:BenWa) 2012-04-13 13:36:29 PDT
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 (⌚ UTC+1/+2) 2012-04-14 06:41:34 PDT

Note You need to log in before you can comment on or make changes to this bug.