New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 692369 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

mojo::Channel::ReadBuffer keep memory allocated all the time

Project Member Reported by ssid@chromium.org, Feb 15 2017

Issue description

On Android, ReadBuffer objects always allocated even if the application is left untouched for minutes (expecting no IPC to happen).
This keeps around 256KiB of memory always, 4 buffers each of 64KiB. Allocated by the stack trace:

[Thread: Chrome_IOThread]
</system/lib/libc.so>
ThreadFunc
base::Thread::ThreadMain()
content::BrowserThreadImpl::Run(base::RunLoop*)
content::BrowserThreadImpl::IOThreadRun(base::RunLoop*)
base::RunLoop::Run()
base::MessageLoop::RunHandler()
base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)
event_base_loop
base::MessagePumpLibevent::OnLibeventNotification(int, short, void*)
OnFileCanReadWithoutBlocking
mojo::edk::Channel::OnReadComplete(unsigned int, unsigned int*)
base::AlignedAlloc(unsigned int, unsigned int)


1. Can we clear the memory when not used?
2. Or reduce the buffer length for Android?
3. Only 4 buffers when taking a snapshot at any point. Can this number grow higher?

Attached a trace file.
 
android_mojo_read_buffer.json
6.7 MB View Download

Comment 1 by roc...@chromium.org, Feb 15 2017

Cc: -roc...@chromium.org
Owner: roc...@chromium.org
Status: Assigned (was: Untriaged)
Sure, I can take a look. If this memory usage is browser process only, I'm assuming there must be 4 child processes in this scenario (and if it's global, then 2 child processes). For any given child process, there's a single read buffer in the browser process and a single read buffer in the child process. The number can obviously grow higher if there are more child processes.

Some background:

The read buffer is a place we use for incoming message data from another process, and for any single message it may grow as large as 256 MB. After a complete message is read and dispatched we immediately shrink the buffer back down to some maximum unused capacity (currently 64 kB) to avoid wasting memory while also avoiding allocation for the vast majority of subsequent reads.

The simplest solution here would be to reduce the max unused buffer capacity to be as small as possible without substantially increasing the frequency of allocations needed to read messages. It's likely that the ideal size is much smaller than 64 kB and we just haven't bothered to measure.

Comment 2 by ssid@chromium.org, Feb 15 2017

Yes. it is seen on each renderer as 64KiB. Yes there are 4 child processes 2 sites, 1 blank and 1 gpu. So, a total of 512Kib is used across all processes for the read buffer.

Also it might be good to investigate why we could go upto 256MiB message on Android. I will look into this.

Comment 3 by roc...@chromium.org, Feb 15 2017

I only meant that the code at this layer is written to allow messages to be
that long. In practice we have a stricter limit on message length
elsewhere, and I doubt we even approach that limit anyway.

Comment 4 by ssid@chromium.org, Feb 15 2017

Cc: haraken@chromium.org
Okay, just tried to open 15 tabs and this buffers grew to 1Mib in browser and 1MiB all renderers. Maybe we should try to clear these for background tabs?

Comment 5 by roc...@chromium.org, Feb 15 2017

I don't think it makes sense to plumb any browser state all the way down to
this layer. Again, all buffers should shrink back down to 64 kB ASAP once
unused, and we should be able to find a more suitable number for that state.
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/adb217e7938f1f84c3f528bec160f9cfa35fb74d

commit adb217e7938f1f84c3f528bec160f9cfa35fb74d
Author: rockot <rockot@chromium.org>
Date: Fri Feb 17 04:30:30 2017

Mojo EDK: Reduce unused read buffer capacity

Reduces the unused read buffer capacity from 64k to 4k.

A simple empirical measurement shows that ~90% of all
messages received by the browser in a "typical" browsing
session are no larger than 4k, so the extra allocations here
should not have a significant performance impact.

BUG= 692369 

Review-Url: https://codereview.chromium.org/2701513005
Cr-Commit-Position: refs/heads/master@{#451209}

[modify] https://crrev.com/adb217e7938f1f84c3f528bec160f9cfa35fb74d/mojo/edk/system/channel.cc

Comment 7 by roc...@chromium.org, Feb 17 2017

Status: Fixed (was: Assigned)
This should be taken care of. Please let me know if you think it's still problematic to keep a 4kB buffer lying around for each IPC endpoint.

Sign in to add a comment