1464773 - Add low-memory event counts to the crash ping

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

+++ This bug was initially created as a clone of Bug #1450993 +++

Since landing bug 1451005 very few LOW_MEMORY_EVENTS_COMMIT_SPACE samples have been reported, this can mean two things: either we're not detecting the low-commit space condition or we're crashing nonetheless so we never get a main ping with the samples populated. To distinguish between the two we should augment the crash ping to also contain the counts for low-memory events so that we can distinguish between the two scenarios.

Comment 1

[Gabriele Svelto [:gsvelto] (PTO)]

Oops, I forgot to change the bug title.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Bug 1464773 - Add low-memory event counts to the crash report; .mielczarek

Review commit: https://reviewboard.mozilla.org/r/247220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/247220/

Comment 3

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8981131 [details]
Bug 1464773 - Add low-memory event counts to the crash report; .mielczarek

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247220/diff/1-2/

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8981131 [details]
Bug 1464773 - Add low-memory event counts to the crash report; .mielczarek

https://reviewboard.mozilla.org/r/247220/#review254122

::: xpcom/base/AvailableMemoryTracker.cpp:362
(Diff revision 2)
>    if ((kLowCommitSpaceThreshold != 0) &&
>        (aStat.ullAvailPageFile < kLowCommitSpaceThreshold)) {
>      sNumLowCommitSpaceEvents++;
> +    CrashReporter::AnnotateCrashReport(
> +      NS_LITERAL_CSTRING("LowCommitSpaceEvents"),
> +      nsPrintfCString("%" PRIu32, uint32_t(sNumLowCommitSpaceEvents)));

I wonder if at some point in the future we could change `AnnotateCrashReport` to have an overload that took integer types directly and wrote them out using the existing integer serialization code in nsExceptionHandler.cpp? We currently have a few special-case things that get that treatment. It would improve the ergonomics a bit. (Followup fodder, to be clear.)

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8981131 [details]
Bug 1464773 - Add low-memory event counts to the crash report; .mielczarek

https://reviewboard.mozilla.org/r/247220/#review254122

> I wonder if at some point in the future we could change `AnnotateCrashReport` to have an overload that took integer types directly and wrote them out using the existing integer serialization code in nsExceptionHandler.cpp? We currently have a few special-case things that get that treatment. It would improve the ergonomics a bit. (Followup fodder, to be clear.)

I added that in bug 1348273, unfortunately I still cannot land it :( Once that lands I'll convert this to use the overload so that we don't have this much duplicated code.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb19935b1930
Add low-memory event counts to the crash report; r=ted.mielczarek

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/fb19935b1930