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

RESOLVED FIXED in mozilla23

Status

()

Core
Gecko Profiler
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jseward, Assigned: jseward)

Tracking

Trunk
mozilla23
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
So that we can get some idea, for nightlies, whether Breakpad is
having success with CFI unwinds, FP unwinds, etc.
(Assignee)

Comment 1

5 years ago
Created attachment 739576 [details] [diff] [review]
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.
Attachment #739576 - Flags: review?(bgirard)
(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 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.
Attachment #739576 - Flags: review?(bgirard) → review+
https://hg.mozilla.org/mozilla-central/rev/47cc840de9d1
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.