Add a DiscardableMemoryShmemeAllocator.

mojo/services/html_viewer/discardable_memory_allocator.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/services/html_viewer/discardable_memory_allocator.h"
+
+#include "base/memory/discardable_memory.h"
+#include "base/memory/weak_ptr.h"
+#include "base/stl_util.h"
+
+namespace html_viewer {
+
+// Represents an actual memory chunk. This is an object owned by
+// DiscardableMemoryAllocator. DiscardableMemoryChunkImpl are passed to the
+// rest of the program, and access this memory through a weak ptr.
+class DiscardableMemoryAllocator::OwnedMemoryChunk {
+ public:
+  explicit OwnedMemoryChunk(size_t size)
+      : is_locked_(true),
+        size_(size),
+        memory_(new uint8_t[size]),
+        allocator_(nullptr),

[jamesr, 2015/03/18 22:40:14]

does it every make sense for allocator_ to be null or to change? seems like it
should be a constructor param if not (and i can't see how it would)

+        weak_factory_(this) {}
+  ~OwnedMemoryChunk() {}
+
+  void Lock() {
+    DCHECK(!is_locked_);
+    is_locked_ = true;
+
+    // While locked, we also move ourselves to the back of the list.
+    allocator_->MoveToBack(it_);
+  }
+
+  void Unlock() {
+    DCHECK(is_locked_);
+    is_locked_ = false;
+  }
+
+  bool locked() const { return is_locked_; }
+  size_t size() const { return size_; }
+  void* Memory() const {
+    DCHECK(is_locked_);
+    return memory_.get();
+  }
+
+  base::WeakPtr<OwnedMemoryChunk> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  }
+
+  void SetAllocatorAndPosition(std::list<OwnedMemoryChunk*>::iterator it,
+                               DiscardableMemoryAllocator* allocator) {
+    it_ = it;
+    allocator_ = allocator;
+  }
+
+ private:
+  bool is_locked_;
+  size_t size_;
+  scoped_ptr<uint8_t[]> memory_;
+
+  std::list<OwnedMemoryChunk*>::iterator it_;
+  DiscardableMemoryAllocator* allocator_;
+
+  base::WeakPtrFactory<OwnedMemoryChunk> weak_factory_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(OwnedMemoryChunk);
+};
+
+namespace {
+
+// Interface to the rest of the program. These objects are owned outside of the
+// allocator and wrap a weak ptr.
+class DiscardableMemoryChunkImpl : public base::DiscardableMemory {
+ public:
+  explicit DiscardableMemoryChunkImpl(
+      base::WeakPtr<DiscardableMemoryAllocator::OwnedMemoryChunk> chunk)
+      : memory_chunk_(chunk) {}
+  ~DiscardableMemoryChunkImpl() override {}
+
+  // Overridden from DiscardableMemoryChunk:
+  bool Lock() override {
+    if (!memory_chunk_)
+      return false;
+
+    memory_chunk_->Lock();
+    return true;
+  }
+
+  void Unlock() override {
+    if (memory_chunk_)

[jamesr, 2015/03/18 22:40:14]

seems like this should be a DCHECK since it's not valid to call Unlock() unless
the memory is locked and it's not valid for memory_chunk_ to go null while the
memory is locked

+      memory_chunk_->Unlock();
+  }
+
+  void* Memory() const override {
+    if (memory_chunk_)
+      return memory_chunk_->Memory();
+    return nullptr;
+  }
+
+ private:
+  base::WeakPtr<DiscardableMemoryAllocator::OwnedMemoryChunk> memory_chunk_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(DiscardableMemoryChunkImpl);
+};
+
+}  // namespace
+
+DiscardableMemoryAllocator::DiscardableMemoryAllocator(size_t max_memory)
+    : max_memory_(max_memory),
+      total_live_memory_(0) {

[jamesr, 2015/03/18 22:40:14]

0u

+}
+
+DiscardableMemoryAllocator::~DiscardableMemoryAllocator() {
+  STLDeleteElements(&live_chunks_);
+}
+
+scoped_ptr<base::DiscardableMemory>
+DiscardableMemoryAllocator::AllocateLockedDiscardableMemory(size_t size) {
+  OwnedMemoryChunk* chunk = new OwnedMemoryChunk(size);
+  auto it = live_chunks_.insert(live_chunks_.end(), chunk);
+  chunk->SetAllocatorAndPosition(it, this);
+  total_live_memory_ += size;
+
+  // Purge unlocked chunks from the cache until |total_live_memory_| is under

[jamesr, 2015/03/18 22:40:14]

what if the size of all currently locked chunks exceeds the desired maximum
size?

+  // the desired maximum size.
+  it = live_chunks_.begin();
+  while (total_live_memory_ > max_memory_ && it != live_chunks_.end()) {

[jamesr, 2015/03/18 22:40:14]

the second clause here looks like we'll break even if we're over the limit if
everything is locked, but the comment says otherwise. could this instead be a
for loop that iterates through the link_chunks_ at most once evicting as it
goes, if possible?

[Elliot Glaysher, 2015/03/18 23:54:44]

I'm not sure what you're asking for here. I get that the comment is
ambiguous--I've fixed that up. But in the case where all chunks are locked,
there's nothing really that we can do. I've renamed the variable to
desired_max_memory_ to emphasize that this is more of a soft guideline.

If you're asking to do a for loop across the entire linked list, unallocating
all unlocked chunks, we don't want to do that because now we aren't an effective
cache--we want to hold unlocked chunks and only free them on memory pressure.

(However, in the case where we blow through our desired memory limit, and have
our hands tied because all memory is locked, I'd argue that our caller is not
obeying the interface; they need to unlock memory once they're done with it.)

+    if (!(*it)->locked()) {
+      total_live_memory_ -= (*it)->size();
+      delete *it;
+      it = live_chunks_.erase(it);
+    } else {
+      it++;
+    }
+  }
+
+  return make_scoped_ptr(new DiscardableMemoryChunkImpl(chunk->GetWeakPtr()));
+}
+
+void DiscardableMemoryAllocator::MoveToBack(
+    std::list<OwnedMemoryChunk*>::iterator it) {
+  OwnedMemoryChunk* chunk = *it;
+  live_chunks_.erase(it);
+  it = live_chunks_.insert(live_chunks_.end(), chunk);
+  chunk->SetAllocatorAndPosition(it, this);
+}
+
+}  // namespace html_viewer