Open Bug 1639922 Opened 5 years ago Updated 3 years ago

Add a memory reporter for protocols and channels

Categories

(Core :: IPC, task)

task

Tracking

()

People

(Reporter: mccr8, Unassigned)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [MemShrink:P2])

Attachments

(2 files)

I'm seeing about 258KB of unreported memory in a content process with stacks with things like IPC::Channel::Channel(), IPC::Channel::ChannelImpl::Send(), and mozilla::ipc::IToplevelProtocol::IToplevelProtocol() in them. It would be nice if we had memory reporters because it will help us understand content process memory overhead.

Do you have a log with the stacks? It might help for understanding the scope of what needs to be reported.

Flags: needinfo?(continuation)
Attached file IPC unreported memory (deleted) —

Here's some of the top hits filtering for allocation stacks which contain "IPC" in the top 4 frames. Some of this is things allocated when we created a protocol, so not IPC, but most of it looks like actual internal IPC stuff.

Flags: needinfo?(continuation)

My patch in bug 1641090 gets rid of the std hash_table stuff under IToplevelProtocol::IToplevelProtocol.

49,152 bytes (34,656 requested / 14,496 slop)
Individual block sizes: 12,288 x 4
#01: IPC::Channel::Channel(int, IPC::Channel::Mode, IPC::Channel::Listener*) (/Users/andrewmccreight/mc/ipc/chromium/src/chrome/common/ipc_channel_posix.cc:927)

This looks like ChannelImpl, which has an inline buffer for reads and another one for cmsg data.

There's not much to be done about those (although moving them out of line might reduce the clownshoes lossage suggested by a 12k size and 2x4k buffers, and allow dynamically resizing them). It looks like the findings are mostly ChannelImpl allocations and the std:: containers you already looked at.

Good point about the slop. I didn't notice that. Yeah, the containers are really the only obvious issue here, but it would still be nice to have memory reporters if it was easily doable somehow.

Right now the total size of IPC::Channel::ChannelImpl on 64-bit Linux is 8672; with the current WIP on bug 1643732 it would become 5088.

The other thing about ChannelImpl is that there are two per channel (in different processes), and from the ipc-channels tree in about:memory each content process seems to have 11-12 channels. So it's worth trying to trim a few kB from these if it can be done easily.

Good point! I haven't thought about the issue of parent process memory usage that scales linearly with the number of processes, but that's important to look at, too.

As for the original purpose of this bug: one problem with adding reporters is that I don't know if we have a list of all the channels (or toplevels) in a process. (The ipc-channels{,-peak} reporter works by keeping counts in a global hashtable, so it would be possible to report some of the overhead based on those counts, but there's no access to the objects themselves to get the current state of the queues, for example.)

Assignee: nobody → continuation

I think most of the top-level protocols are just stored in a static variable somewhere, so if I write a SizeOf method for ITopLevelProtocol, I could probably write a few lines of boilerplate for each top-level protocol to get them set up for memory reporting.

Kashav said he could take over. I'm a bit too busy with memory reporting and AWSY stuff to work on this soon.

Assignee: continuation → kmadan

Andrew said he'll be able to get back to this. I didn't have much, but I've posted my changes at https://phabricator.services.mozilla.com/D87877.

Assignee: kmadan → continuation
Assignee: continuation → nobody

Bug 1729890 is a case where some kind of parent process leak caused us to accumulate thousands of HttpChannelParents which only showed up as heap-unclassified.

URL: 1729890
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: