Improve Rust OOM signatures
Categories
(Socorro :: Signature, task, P2)
Tracking
(Not tracked)
People
(Reporter: mccr8, Assigned: willkg)
References
Details
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
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.
Assignee | ||
Comment 1•4 years ago
|
||
rust_oom
isn't in the prefix list which is why that second signature ends there.
Seems like we should make the following changes:
-
add
hashbrown::raw::RawTable<T>::new_uninitialized<T>
to the prefix list so signature generation continues -
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
, andrust_oom
to the irrelevant list.
Andrew: Does that summarize what you're suggesting?
Gabriele: Does this make sense to do?
Comment 2•4 years ago
|
||
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.
Reporter | ||
Comment 3•4 years ago
|
||
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.
Assignee | ||
Comment 4•4 years ago
|
||
Makes sense--thank you!
Grabbing this to do soon.
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
Reporter | ||
Comment 7•4 years ago
|
||
Thanks!
Assignee | ||
Comment 8•4 years ago
|
||
Deployed this to prod in bug #1642634. Marking as FIXED.
Description
•