875131 - profile using C++-like stack scanning instead of the SPS psuedo-stack

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect, P3)

Description

[Luke Wagner [:luke]]

Currently, when profiling is enabled, JIT code pushes and pops entries on the SPS pseudo stack.  This makes profiling simple (the profiler just reads the pseudo stack from the signal handler), but this incurs non-trivial overhead (5-10% iirc, maybe more) so we can't leave profiling turned on all the time.

An alternative, that wouldn't incur any runtime overhead, would be to support directly walking the stack from the signal handler.  This is harder and requires we think Real Hard about racing with the signal handler.  I think it's doable, though, and we already perform on-demand $rip-to-line/script mapping (when the signal occurs while executing JIT code).

Taras pointed out a really good use case for doing the no-overhead approach: the hang detector.  What we apparently already do is, when the Gecko event loop is hung on a single event for more than X seconds, we take a stacktrace (using normal C++ stack walking) and send this stack back via Telemetry/FHR.  This is useful for pointing out hangs in Gecko code, but a lot of hang stacks end up going into the JS blackhole.  However, if we supported on-demand JS stack walking, we could also see what JS was running.  This would give us insight into, e.g., Chrome or Addon code that was commonly janking our users.  It'd also be useful to see if there were any popular sites that were janky and, e.g., see if they were only janky in FF.

Comment 1

[Luke Wagner [:luke]]

One obvious problem is that, if we're using the system's builtin stack walker, we won't have a chance to help out when the walking gets to jit frames.  However, from my minimal understanding, we're planning to switch from using the system builtin stack walker to using breakpad's stack walking which means we'd be able to hack it up.  Is that right Julian/Taras?

If so, Julian: what would your ideal interface be between the stack walker and the JS engine?

Comment 2

[Benoit Girard (:BenWa)]

SPS has had two stack scanning implementation:
1) Hand written one. It scanned the stack, looked for addresses pointing to a mapped library and check that it was a proper call site. This was done for arm-android. It never worked better then the pseudostack mode because we often had hold return addresses on the stack. Jeff: 'think large unintialized arrays on the stack that contain old return addresses'.
2) The breakpad stack scanner. The results are very poor thus the samples doesn't aggregate together in the UI. Because of this we don't get any samples with large counts so we don't know where to look at.

HOWEVER, Julian and I have discussed grabbing stack scanning but NOT displaying them in the treeview. When you get a pseudostack site with a large sample count you could right click and reveal the stack scanning for this site. This solves the nonsensical aggregation problem (something I believe even a trained power user would not get used to).

(In reply to Luke Wagner [:luke] from comment #1)
> we're planning to switch
> from using the system builtin stack walker to using breakpad's stack walking
> which means we'd be able to hack it up.  Is that right Julian/Taras?
> 

Yes that's correct I believe and would be very interesting to try.

Comment 3

[Benoit Girard (:BenWa)]

hold/old

Comment 4

[Luke Wagner [:luke]]

What makes the samples poor with breakpad?  If it is jit code on the stack, then we could be fixing that :)

Comment 5

[Benoit Girard (:BenWa)]

(In reply to Luke Wagner [:luke] from comment #4)
> What makes the samples poor with breakpad?  If it is jit code on the stack,
> then we could be fixing that :)

Even with JIT code aside (say running JS Interepter only using prefs) un-initialized memory containing stale return addresses throw off good stack scanning heuristics.

Comment 6

[Luke Wagner [:luke]]

Oh, I thought we used debug/dwarf info to know the precise size of stack frames.  Is that only on some platforms?

Comment 7

[Benoit Girard (:BenWa)]

I was trying to point out that stack scanning showed to be very poor for c++ because of the reason I mention above. Perhaps these stack scanning heuristics would work better for jitted frame.

Comment 8

[Luke Wagner [:luke]]

Ok, so I think you've explained why (1) in comment 2 had poor results, which makes sense.  I was asking why in (2), where presumably you have precise stack frame information (modulo jit code, which I was saying in comment 4 we could fix here), the results were poor.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Luke Wagner [:luke] from comment #8)
> Ok, so I think you've explained why (1) in comment 2 had poor results, which
> makes sense.  I was asking why in (2), where presumably you have precise
> stack frame information (modulo jit code, which I was saying in comment 4 we
> could fix here), the results were poor.

I think you're just both not talking about the same thing when you say "stack scanning". What Benoit is talking about is actually scanning the stack contents to attempt finding frames when you don't have debug info.

Comment 10

[Benoit Girard (:BenWa)]

Yes, I've been using "stack scanning" to mean inferring the stack without deterministic rules for reconstructing it by using custom inexact heuristics and "stack unwinding" to mean reconstructing the stack using guaranteed information (ABI, unwind info).

Comment 11

[Luke Wagner [:luke]]

Ah, I assumed that when you said "breakpad stack scanner" in (2) in comment 2, you were talking about "stack unwinding" (viz., I thought breakpad did stack unwinding, but that is from old and probably mis-remembered conversations with jimb).

So, am I right that the profiler/breakad *never* do unwinding, only scanning?  (Do any of our tools unwind?)  In that case, it sounds like all the profiler needs from the JIT is a list of (symbol, address ranges) pairs?  Anything else?

Comment 12

[Luke Wagner [:luke]]

With all of djvj's work in the intervening years, I think this bug is mostly WFM, although, as a separate, more focused bug, it would simplify everything if we could have the ProfilingFrameIterator traverse InterpreterActivations instead of having the interpreter push magic isJs psuedo-frames.