1640942 - Improve Rust OOM signatures

Status: RESOLVED FIXED

(Socorro :: Signature, task, P2)

Description

[Andrew McCreight [:mccr8]]

Two recent bugs, bug 1640564 and bug 1639996, have shown that we have pretty unpleasant signature for Rust OOMs.

[@ OOM | large | mozalloc_abort | mozalloc_handle_oom | gkrust_shared::oom_hook::hook | std::alloc::rust_oom | hashbrown::raw::RawTable<T>::new_uninitialized<T>]

[@ OOM | large | mozalloc_abort | <name omitted> | gkrust_shared::oom_hook::hook | rust_oom]

mozalloc_handle_oom, gkrust_shared::oom_hook::hook, std::alloc::rust_oom, and rust_oom all look like boilerplate that should be added to the skip list. I guess most of these are in the prefix list already, because they are getting the frame past them, but it doesn't seem that useful.

mozalloc_abort might also be nice to remove, but I'm not sure where else it might show up.

RawTable<T>::new_uninitialized should probably also be added to the prefix list, so we can split signatures depending on the places that are creating the hash tables.

I don't really have time at the moment to dig into this issue further, but I wanted to get a bug filed for this.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

rust_oom isn't in the prefix list which is why that second signature ends there.

Seems like we should make the following changes:

1. add hashbrown::raw::RawTable<T>::new_uninitialized<T> to the prefix list so signature generation continues

2. then either add rust_oom to the prefix list so signature generation continues,

   OR add mozalloc_handle_oom, gkrust_shared::oom_hook::hook, std::alloc::rust_oom, and rust_oom to the irrelevant list.

Andrew: Does that summarize what you're suggesting?

Gabriele: Does this make sense to do?

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

Makes sense. We've got a lot of OOM-related functions in the prefix list so I wonder if the irrelevant list option is the best one. Having a super-long signature is not really helpful especially in an OOM crash that is usually already clearly marked as such.

Comment 3

[Andrew McCreight [:mccr8]]

Yes, that matches what I'm thinking of. mozalloc_abort could probably go too, though I do see some signatures like [@ mozalloc_abort | sk_abort_no_print | SkGlyphRunListPainter::drawForBitmapDevice ] where this is really just a MOZ_CRASH, so maybe we can leave it alone. Just one junky-ish frame isn't too bad.

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

Makes sense--thank you!

Grabbing this to do soon.

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

Attachment: pr 5402: bug 1640942: improve rust OOM signatures

Comment 6

[Will Kahn-Greene [:willkg] ET needinfo? me]

willkg merged PR #5402: "bug 1640942: improve rust OOM signatures" in c1947ef.

Comment 7

[Andrew McCreight [:mccr8]]

Thanks!

Comment 8

[Will Kahn-Greene [:willkg] ET needinfo? me]

Deployed this to prod in bug #1642634. Marking as FIXED.