1882088 - Improve nsTArray OOM signatures

Status: RESOLVED FIXED

(Socorro :: Signature, task)

Description

[Andrew McCreight [:mccr8]]

I came across a few OOMs while appending to an array today (bug 1851054 and bug 1882082), and their signatures are really bad because they are so long. I'm not even sure what the full signature is. Some of these internal methods should get moved to the irrelevant signature list.

[@ OOM | large | mozalloc_abort | moz_xrealloc | nsTArrayInfallibleAllocator::Realloc | nsTArray_base<T>::EnsureCapacityImpl<T> | nsTArray_base<T>::EnsureCapacity | nsTArray_Impl<T>::AppendElementInternal | nsTArray<T>::AppendElement | nsAttrValue::ParseA... ]

[@ OOM | large | mozalloc_abort | moz_xrealloc | nsTArrayInfallibleAllocator::Realloc | nsTArray_base<T>::EnsureCapacityImpl<T> | nsTArray_base<T>::EnsureCapacity | nsTArray_Impl<T>::AppendElementInternal | nsTArray<T>::AppendElement | mozilla::MediaTrack... ]

• mozalloc_abort looks fairly useless, except in cases where we seem to fail to figure out that it is an OOM and end up with a signature like this: [@ mozalloc_abort | XPCJSContext::NewXPCJSContext ] . See: bp-06150eaa-bff5-46dc-8d5e-6aa120240226. Maybe that's not enough of a reason to keep it around, unless the OOM signature thing is relying on it.
• moz_xrealloc is basically the same story, appearing only in OOMs or things that should be OOMs.
• nsTArrayInfallibleAllocator is in the prefix list, but I think should be in the irrelevant list.
• nsTArray_base should probably get moved from the prefix list to the irrelevant list. Looking at the top 50 signatures with nsTArray_base in them, only one or two didn't have nsTArray_Impl in there, and I think they would be clear enough without that (eg it was a length method calling nsTArray_base::Length, or an append method calling into the nsTArray_base appending stuff)
• nsTArray_Impl can be left as-is. There are a few cases where we have multiple frames of it (eg nsTArray_Impl<T>::ElementAt | nsTArray_Impl<T>::operator[] or nsTArray_Impl<T>::ClearAndRetainStorage | nsTArray_Impl<T>::Clear) where it isn't too useful, but that's probably better dealt with via doing an irrelevant signature for a specific method or maybe via a sentinel.

Comment 1

[Andrew McCreight [:mccr8]]

I'll take a look, though this requires editing multiple files so it is a bit fancier than most of the changes I make here.

Comment 2

[Andrew McCreight [:mccr8]]

Here are some example reports for the two bugs I mentioned:
bp-b82f35b2-99f4-48da-85a2-c9ec20240226
bp-74a0e54f-729f-410f-ae14-a70920240226

Comment 3

[Andrew McCreight [:mccr8]]

I'll also move over moz_xmalloc. There are a few signatures like [@ mozalloc_abort | moz_xmalloc | new ] that will get weird, but as with xrealloc, these are mostly OOM signatures.

I added InfallibleAllocPolicy to the prefix list, to make signatures like [@ mozalloc_abort | moz_xmalloc | InfallibleAllocPolicy::pod_malloc ] more useful. Maybe it makes more sense as an irrelevant signature, but the volume is fairly low and I don't see crashes with a ton of frames. example: bp-a255024f-64ec-40f8-8ba5-1729b0240226

Comment 4

[Andrew McCreight [:mccr8]]

https://github.com/mozilla-services/socorro/pull/6534

Comment 5

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

Attachment: [mozilla-services/socorro] Bug 1882088 - Move nsTArray OOM things to the irrelevant list. (#6534)

Comment 6

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

willkg merged PR [mozilla-services/socorro]: Bug 1882088 - Move nsTArray OOM things to the irrelevant list. (#6534) in 84f34a4.

Comment 7

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

This deployed to prod in bug #1882847. Marking as FIXED.