863705 - SPS breakpad: show frame-trust statistics in the debugging log

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Julian Seward [:jseward]]

So that we can get some idea, for nightlies, whether Breakpad is
having success with CFI unwinds, FP unwinds, etc.

Comment 1

[Julian Seward [:jseward]]

Attachment: Patch

I think this would be a useful addition for troubleshooting unwinding
in nightlies.  It generates lines like this:

Profiler: BPUnw frame stats: \
  TOTAL  4000    CTX  224    CFI  540    FP 3236    SCAN    0    NONE    0

TOTAL is the total number of frames in this sample interval (changed to 5000
      in the patch)
CTX   is # frames from direct context (iow direct-from-CPU program counter)
CFI   is # frames from CFI
FP    is # frames from FP chasing
SCAN  is # frames from stack scan
NONE  is # frames with some other trustworthyness metric (should always be zero)

Also, because there is always exactly one CTX frame per unwind, the average
unwind length is TOTAL / CTX.

Comment 2

[Benoit Girard (:BenWa)]

(In reply to Julian Seward from comment #1)
> Also, because there is always exactly one CTX frame per unwind

That's neat. That tell you the number of samples.

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 739576 [details] [diff] [review]
Patch

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

I'm happy with the patch as-is but I'd like to discuss my second comment before landing.

::: tools/profiler/UnwinderThread2.cpp
@@ +1550,5 @@
> +      nf_CFI_SCAN++; break;
> +    case google_breakpad::StackFrame::FRAME_TRUST_FP: nf_FP++; break;
> +    case google_breakpad::StackFrame::FRAME_TRUST_CFI: nf_CFI++; break;
> +    case google_breakpad::StackFrame::FRAME_TRUST_CONTEXT: nf_CONTEXT++; break;
> +    default: break;

Perhaps default should count towards NONE. Not a big deal since we can always do TOTAL - SUM(...).

@@ +1712,5 @@
>        if (n_frames_dubious > (unsigned int)sUnwindStackScan)
>          break;
>  
> +      if (LOGLEVEL >= 2)
> +        stats_notify_frame(frame->trust);

Perhaps we should do this before we discard stack scan? This way we can watch SCAN/NONE for incomplete unwinds.

Comment 4

[Julian Seward [:jseward]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/47cc840de9d1

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47cc840de9d1