base: Add free list implementation to browser-wide discardable memory system.

base: Add free list implementation to browser-wide discardable memory system.

This adds a free list to the browser-wide discardable memory
manager in child processes. This reduces the number of open
file descriptors and improves performance significantly by
avoiding a lot of browser process round-trips.

Address-ordered best-fit policy is currently used as it is more
likely to result in fewer locked segments but the policy can
easily be adjusted if needed.

BUG=429415
TEST=content_unittests --gtest_filter=DiscardableSharedMemoryHeapTest.*

Committed: https://crrev.com/94ff17c7b04f1e3abfb33c91a2e8ebb70b6882ca
Cr-Commit-Position: refs/heads/master@{#309595}

[reveman, 2014-12-17 21:58:59]

reveman@chromium.org changed reviewers:
+ avi@chromium.org

[reveman, 2014-12-17 21:58:59]

Please take a look.

[Avi (use Gerrit), 2014-12-18 20:49:07]

lgtm

Nits, really.

https://codereview.chromium.org/807303002/diff/60001/content/common/discardab...
File content/common/discardable_shared_memory_heap.h (right):

https://codereview.chromium.org/807303002/diff/60001/content/common/discardab...
content/common/discardable_shared_memory_heap.h:60: // Merge |span| into the
free list. This will coalesced |span| with
coalesce

https://codereview.chromium.org/807303002/diff/60001/content/common/discardab...
content/common/discardable_shared_memory_heap.h:64: // Split an allocated span
into two spans. One of length |blocks| followed
"... into two spans, one of length ..."

https://codereview.chromium.org/807303002/diff/80001/content/common/discardab...
File content/common/discardable_shared_memory_heap.cc (right):

https://codereview.chromium.org/807303002/diff/80001/content/common/discardab...
content/common/discardable_shared_memory_heap.cc:103: Span* best = NULL;
nullptr

https://codereview.chromium.org/807303002/diff/80001/content/common/discardab...
content/common/discardable_shared_memory_heap.cc:151: // No need to coalesced as
the previous span of |leftover| was just split
s/coalesced/coalesce/

[reveman, 2014-12-18 21:30:43]

reveman@chromium.org changed reviewers:
+ danakj@chromium.org

[reveman, 2014-12-18 21:30:44]

+danakj for base/

To give both of you an idea of where all this is headed, here's what I'm going
to do next:

1. enable this code by default on win,linux,cros
2. remove old emulated discardable memory code
3. enable this code by default on android
4. remove old ashmem discardable memory code (yay! same code on all platforms
but mac)
5. remove run-time config for discardable memory impl
6. remove all DiscardableMemoryManager related code from base/ (I'll keep a much
simpler and less generic version of this in discardable_memory_mac.cc)
7. investigate if we should be using browser-wide discardable memory on mac too

https://codereview.chromium.org/807303002/diff/60001/content/common/discardab...
File content/common/discardable_shared_memory_heap.h (right):

https://codereview.chromium.org/807303002/diff/60001/content/common/discardab...
content/common/discardable_shared_memory_heap.h:60: // Merge |span| into the
free list. This will coalesced |span| with
On 2014/12/18 20:49:06, Avi wrote:
> coalesce

Done.

https://codereview.chromium.org/807303002/diff/60001/content/common/discardab...
content/common/discardable_shared_memory_heap.h:64: // Split an allocated span
into two spans. One of length |blocks| followed
On 2014/12/18 20:49:06, Avi wrote:
> "... into two spans, one of length ..."

Done.

https://codereview.chromium.org/807303002/diff/80001/content/common/discardab...
File content/common/discardable_shared_memory_heap.cc (right):

https://codereview.chromium.org/807303002/diff/80001/content/common/discardab...
content/common/discardable_shared_memory_heap.cc:103: Span* best = NULL;
On 2014/12/18 20:49:06, Avi wrote:
> nullptr

Done.

https://codereview.chromium.org/807303002/diff/80001/content/common/discardab...
content/common/discardable_shared_memory_heap.cc:151: // No need to coalesced as
the previous span of |leftover| was just split
On 2014/12/18 20:49:06, Avi wrote:
> s/coalesced/coalesce/

Done.

[danakj, 2014-12-18 21:40:50]

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem.cc (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem.cc:110: DCHECK(!is_locked_);
Why no chunk_->Purge()?

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem.h (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem.h:43:
scoped_ptr<DiscardableMemoryShmemChunk> chunk_;
What is the point of wrapping DiscardableSharedMemory in this new interface?

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem_allocator.cc (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem_allocator.cc:44: return
scoped_ptr<DiscardableMemoryShmemChunk>();
return nullptr

[reveman, 2014-12-19 00:49:54]

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem.cc (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem.cc:110: DCHECK(!is_locked_);
On 2014/12/18 21:40:50, danakj wrote:
> Why no chunk_->Purge()?

The underlying shared memory can't be purged as it might be used by another
chunk. Deleting the chunk instance results in the behavior expected by the user
of this API. The memory is returned to the free list and a new chunk needs to be
allocated next time AllocateAndAcquireLock() is called.

Note: ::Purge() API will be removed when the Chunk class is removed.

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem.h (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem.h:43:
scoped_ptr<DiscardableMemoryShmemChunk> chunk_;
On 2014/12/18 21:40:50, danakj wrote:
> What is the point of wrapping DiscardableSharedMemory in this new interface?

Each DiscardableSharedMemory instance is used for multiple DiscardableMemory
instances. We have X number of DiscardableSharedMemory instances and Y number of
DiscardableMemoryShmemChunk instances for each DiscardableSharedMemory instance.

DiscardableMemoryShmemChunk will eventually be removed and we'll just have X
DiscardableSharedMemory and Y DiscardableMemory instances. See the comment in
discardable_memory_shmem_allocator.h. I just need to first enable this code and
remove the old emulated discardable memory system before I can do this.

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem_allocator.cc (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem_allocator.cc:44: return
scoped_ptr<DiscardableMemoryShmemChunk>();
On 2014/12/18 21:40:50, danakj wrote:
> return nullptr

Done. Here and a few other places.

[danakj, 2014-12-19 21:08:09]

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem.h (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem.h:43:
scoped_ptr<DiscardableMemoryShmemChunk> chunk_;
On 2014/12/19 00:49:54, reveman wrote:
> On 2014/12/18 21:40:50, danakj wrote:
> > What is the point of wrapping DiscardableSharedMemory in this new interface?
> 
> Each DiscardableSharedMemory instance is used for multiple DiscardableMemory
> instances. We have X number of DiscardableSharedMemory instances and Y number
of
> DiscardableMemoryShmemChunk instances for each DiscardableSharedMemory
instance.
> 
> DiscardableMemoryShmemChunk will eventually be removed and we'll just have X
> DiscardableSharedMemory and Y DiscardableMemory instances. See the comment in
> discardable_memory_shmem_allocator.h. I just need to first enable this code
and
> remove the old emulated discardable memory system before I can do this.

Sorry this explanation confuses me. Specifically DiscardableSharedMemory is
created and put in a scoped_ptr, then Passed to and wrapped in a new scoped_ptr
inside a DiscardableMemoryShmemChunkImpl. So these seem to be 1:1. So what do
you mean by there are X DiscardableSharedMemory and Y
DiscardableMemoryShmemChunk?

[reveman, 2014-12-19 22:16:03]

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
File base/memory/discardable_memory_shmem.h (right):

https://codereview.chromium.org/807303002/diff/120001/base/memory/discardable...
base/memory/discardable_memory_shmem.h:43:
scoped_ptr<DiscardableMemoryShmemChunk> chunk_;
On 2014/12/19 21:08:08, danakj wrote:
> On 2014/12/19 00:49:54, reveman wrote:
> > On 2014/12/18 21:40:50, danakj wrote:
> > > What is the point of wrapping DiscardableSharedMemory in this new
interface?
> > 
> > Each DiscardableSharedMemory instance is used for multiple DiscardableMemory
> > instances. We have X number of DiscardableSharedMemory instances and Y
number
> of
> > DiscardableMemoryShmemChunk instances for each DiscardableSharedMemory
> instance.
> > 
> > DiscardableMemoryShmemChunk will eventually be removed and we'll just have X
> > DiscardableSharedMemory and Y DiscardableMemory instances. See the comment
in
> > discardable_memory_shmem_allocator.h. I just need to first enable this code
> and
> > remove the old emulated discardable memory system before I can do this.
> 
> Sorry this explanation confuses me. Specifically DiscardableSharedMemory is
> created and put in a scoped_ptr, then Passed to and wrapped in a new
scoped_ptr
> inside a DiscardableMemoryShmemChunkImpl. So these seem to be 1:1. So what do
> you mean by there are X DiscardableSharedMemory and Y
> DiscardableMemoryShmemChunk?

It's wrapped in a linked_ptr (See
DiscardableSharedMemoryHeap::Span::shared_memory_), so it's ref counted. That
could be wrapped in a RefCounted type instead if linked_ptr is not efficient
enough. I'm not sure that's necessary though.

[danakj, 2014-12-19 22:19:18]

https://codereview.chromium.org/807303002/diff/140001/base/memory/discardable...
File base/memory/discardable_memory_shmem_allocator.cc (right):

https://codereview.chromium.org/807303002/diff/140001/base/memory/discardable...
base/memory/discardable_memory_shmem_allocator.cc:42:
scoped_ptr<DiscardableSharedMemory> memory(new DiscardableSharedMemory);
Where is there a linked_ptr here?

[reveman, 2014-12-19 23:20:26]

https://codereview.chromium.org/807303002/diff/140001/base/memory/discardable...
File base/memory/discardable_memory_shmem_allocator.cc (right):

https://codereview.chromium.org/807303002/diff/140001/base/memory/discardable...
base/memory/discardable_memory_shmem_allocator.cc:42:
scoped_ptr<DiscardableSharedMemory> memory(new DiscardableSharedMemory);
On 2014/12/19 22:19:18, danakj wrote:
> Where is there a linked_ptr here?

This default implementation of DiscardableMemoryShmemAllocator still uses an N-N
mapping between chunks and DiscardableSharedMemory instances. It's used when
content/ is not providing a different allocator implementation. ie. for any
tests that depend on discardable memory but doesn't use content/.

My previous description was referring to the allocator implementation in
content/ that implements a free list.

[danakj, 2014-12-23 17:45:21]

Ohh I see. Sorry I didn't go and read the content/ code I was assuming the base/
stuff was kinda self-contained/explanatory on its own.

LGTM

[reveman, 2014-12-23 17:52:14]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-12-23 17:53:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/807303002/140001

[commit-bot: I haz the power, 2014-12-23 17:59:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-23 17:59:33]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[reveman, 2014-12-23 19:22:35]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-12-23 19:23:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/807303002/160001

[commit-bot: I haz the power, 2014-12-23 21:11:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-23 21:11:31]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[reveman, 2014-12-23 22:33:01]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-12-23 22:33:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/807303002/160001

[commit-bot: I haz the power, 2014-12-24 00:52:58]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2014-12-24 00:53:52]

Patchset 9 (id:??) landed as
https://crrev.com/94ff17c7b04f1e3abfb33c91a2e8ebb70b6882ca
Cr-Commit-Position: refs/heads/master@{#309595}