896753 - Memory reporter for structured clone buffers

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Steve Fink [:sfink] [:s:]]

I swear I typed out this bug description once before, but I don't see it...

Before bug 868700, it was fairly easy to leak structured clone buffers. In the Broadway.js h264 decoder demo, this would give me >90% unclassified-heap bytes. Worse, those bytes would be stored in char*/size_t field pairs, not any sort of structured type.

Bug 868700 keeps the data always in a JSAutoStructuredCloneBuffer, which makes the memory easier to find and also fixes the leak. But it's still be possible to construct structured clones and never deserialize them. Also, if you have postMessages going at a high rate between the main thread and worker threads, you can accumulate quite a few clone buffers. I don't know if it's worth creating a memory reporter for these hypothetical cases.

Comment 1

[Nicholas Nethercote [inactive]]

Where do these buffers end up being held?

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Some of this exists from bug 1382645:
https://searchfox.org/mozilla-central/rev/ef0aa879e94534ffd067a3748d034540a9fc10b0/dom/base/StructuredCloneHolder.h#139

In my case of Worker postMessage I think the StructuredCloneHolderBase ends up being held in the worker event queue and I don't think we try to gather the size of that data.

Comment 3

[Steve Fink [:sfink] [:s:]]

Yeah, that's what I saw too. If possible, I think it would be useful to account for the event queue, because we do see cases where one thread is posting messages faster than they're being consumed, which can result in a lot of memory piling up. (Multiple gigabytes in some cases.) I don't know if there's a backpressure mechanism that is supposed to control this, but if there is then it doesn't seem to be enough when you are Transferring large ArrayBuffers—it only takes a few pending messages to bloat memory up when you're tossing 1000x1000 pixel frames around.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: Demo showing heap unclassified