507249 - Call sites that potentially allocate large amounts of memory should use fallible allocators

Status: RESOLVED WORKSFORME

(Core :: XPCOM, enhancement)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Bug 507082 will land with all Gecko code using the fallible variants of the moz_ allocators.  This bug covers finding the locations of "large" allocations and manually switching them over to moz_alloc or ::operator new(fallible), keeping (or adding) OOM handling.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

bsmedberg suggests that a good way to automatically find these sites is to write a dtrace script that checks the size passed to malloc.  If the size is greater than some threshold, say 1k, then the dtrace would log the size and a few stack frames below the malloc.  Somehow we would incrementally process, or post-process, this data to find the set of unique "large allocation" sites.

Rob, can you tackle this or suggest another dtrace ninja who might?

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

You don't need to be a dtrace ninja to do this, I'm pretty sure, and it would be a good introduction to using the tool.  A newsgroup or blog call for help might well turn up someone who wants to spend an hour or three learning dtrace and gathering some initial data.

Comment 3

[Nicholas Nethercote [inactive]]

It's easy to get stack traces with Valgrind, see http://blog.mozilla.com/nnethercote/2011/01/11/using-valgrind-to-get-stack-traces/.  I can do that and get a list of >1KB allocation sites.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Yes, that would be very useful.

Comment 5

[Shriram (irc: Mavericks)]

It's been a while since Jan'11. Has anyone had a chance to work on this?

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

No updates.  If you would like to tackle this bug, please do!

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We do a good job of figuring this out through crash-stats, so I don't think this bug is useful anymore.

Comment 8

[Daniel Holbert [:dholbert]]

mozalloc_macro_wrappers.h has a reference to this bug, saying:
> 15  * FIXME: use infallible allocators by default after 
> 16  *   http://bugzilla.mozilla.org/show_bug.cgi?id=507249
> 17  * lands.
> 18  */
http://mxr.mozilla.org/mozilla-central/source/memory/mozalloc/mozalloc_macro_wrappers.h

Given comment 7's WORKSFORME resolution (and given karl's synopsis in bug 581478 comment 1), it sounds to me like mozalloc_macro_wrappers.h might be no longer needed and can be removed.  Is that correct?

(If so, that would be great, as its "#define free" is busting some Skia headers which define local functions called "free()" -- see bug 924768.)