1069558 - Switch ThreadStackHelper to use LUL

Status: RESOLVED INACTIVE

(Core :: Gecko Profiler, defect)

Description

[(not currently active) Ted Mielczarek]

ThreadStackHelper uses Breakpad. I'd like to get the Breakpad processor code back out of Firefox since we don't need it for the profiler anymore. Can we switch ThreadStackHelper to use LUL instead?

Comment 1

[(not currently active) Ted Mielczarek]

jchen: does this seem feasible?

Comment 2

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

I chose Breakpad initially over LUL because Breakpad supported all the architectures (x86, x86-64, ARM) that we target. I don't know what LUL supports now, but we can definitely switch to it if it has the same support.

Comment 3

[(not currently active) Ted Mielczarek]

I'm pretty sure LUL supports all of those, given that we're using it as a replacement for Breakpad in SPS. Julian: do I have that right?

Comment 4

[Julian Seward [:jseward]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
It depends which OSs you are referring to.  Currently LUL does indeed unwind on
x86, x86-64 and ARM, but only on Linux(alikes): Linux desktop, Android, B2G
(untested, but should work).

It doesn't work on Win32/64 or OSX.  OSX could be easily doable if we pull
the Mach-O parser from Breakpad into LUL.  Doing Windows would be a whole 
trip though.

Comment 5

[Markus Stange [:mstange]]

We could also wrap the stackwalking that the profiler does into a pretty API that can be called from outside the profiler. Then we'd unwind using whatever code we have available on a given platform; LUL on Linux, frame pointer stackwalking on OS X, StackWalk64 on Windows, EHABI on ARM. The one thing that this wouldn't give us is stackwalking on OS X without frame pointers.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Benoit, jchen and I are talking about what we should do here. One idea we had was moving ThreadStackHelper into tools/ and then moving all of the stackwalking stuff that the profiler has under that. Then we'd make the profiler use ThreadStackHelper. What do you think?

Comment 7

[Benoit Girard (:BenWa)]

I don't think that's a great idea. If we want to provide low level c++ unwinding then the right place to do this is to extend NS_Stackwalk, now MozStackwalk. If we're waiting to provide a clean and powerful highlevel unwind API (like Comment 5 suggests) then the profiler is already exposing 'profiler_get_backtrace' which could use some improvements.

Also the profiler is a standalone project now so I'd rather we didn't rip code out of it unless it provides a very clear benefit.

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Jim what do you think about the two options in comment #7?

Comment 9

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

I like the profiler_get_backtrace option. We'd need to expand it to work with threads other than the current thread.