Enforce a memory limit on MappedMemoryManager

gpu/command_buffer/client/mapped_memory.cc

Index: gpu/command_buffer/client/mapped_memory.cc
diff --git a/gpu/command_buffer/client/mapped_memory.cc b/gpu/command_buffer/client/mapped_memory.cc
index 82829d48ec1cda6fd7534f596b95acbce03b8522..75ed707214857fe1a600ee49a5dd410031e047f8 100644
--- a/gpu/command_buffer/client/mapped_memory.cc
+++ b/gpu/command_buffer/client/mapped_memory.cc
@@ -20,7 +20,17 @@ MemoryChunk::MemoryChunk(
 
 MappedMemoryManager::MappedMemoryManager(CommandBufferHelper* helper)
     : chunk_size_multiple_(1),
-      helper_(helper) {
+      helper_(helper),
+      allocated_memory_(0),
+      memory_limit_(kNoLimit) {
+}
+
+MappedMemoryManager::MappedMemoryManager(CommandBufferHelper* helper,

[no sievers, 2013/08/19 21:57:44]

Let's not add a new constructor for that. It just causes surface for bugs.

[kaanb, 2013/08/20 01:23:43]

Done.

+                                         size_t memory_limit)
+    : chunk_size_multiple_(1),
+      helper_(helper),
+      allocated_memory_(0),
+      memory_limit_(memory_limit) {
 }
 
 MappedMemoryManager::~MappedMemoryManager() {
@@ -36,7 +46,7 @@ void* MappedMemoryManager::Alloc(
     unsigned int size, int32* shm_id, unsigned int* shm_offset) {
   GPU_DCHECK(shm_id);
   GPU_DCHECK(shm_offset);
-  // See if any of the chucks can satisfy this request.
+  // See if any of the chunks can satisfy this request.
   for (size_t ii = 0; ii < chunks_.size(); ++ii) {
     MemoryChunk* chunk = chunks_[ii];
     chunk->FreeUnused();
@@ -49,6 +59,23 @@ void* MappedMemoryManager::Alloc(
     }
   }
 
+  // If there is a memory limit being enforced and the allocated memory
+  // is above the limit, try existing chunks again with waiting.

[epenner, 2013/08/19 22:05:52]

This should be rare right? Should we add a trace around this potentially-slow
path?

[kaanb, 2013/08/20 01:23:43]

See line 76

+  if (memory_limit_ != kNoLimit && allocated_memory_ >= memory_limit_) {

[no sievers, 2013/08/19 21:57:44]

What do you all think about looking at the free+free_pending_token memory here?

That way we can make this a well defined hard limit, for example
max_free_or_pending_memory_ instead of something very vague that you wouldn't
really know what to set it to (you would have to make it dependent on another
value in cc).

Together with the client's limit (i.e. cc's limit for pending raster jobs), this
would then allow us to set a hard limit for all shared memory used for texture
uploads in this path.

You could then probably also safely set it to some high limit for all platforms
and wouldn't have to deal with kNoLimit here.

[kaanb, 2013/08/20 01:23:43]

Done.

+    for (size_t ii = 0; ii < chunks_.size(); ++ii) {
+      MemoryChunk* chunk = chunks_[ii];
+      chunk->FreeUnused();
+      if (chunk->GetLargestFreeSizeWithWaiting() >= size) {
+        void* mem = chunk->Alloc(size);
+        GPU_DCHECK(mem);
+        *shm_id = chunk->shm_id();
+        *shm_offset = chunk->GetOffset(mem);
+        return mem;
+      }
+    }
+    LOG(INFO) << "Exceeding memory limit: " << memory_limit_;
+  }
+
   // Make a new chunk to satisfy the request.
   CommandBuffer* cmd_buf = helper_->command_buffer();
   unsigned int chunk_size =
@@ -59,6 +86,7 @@ void* MappedMemoryManager::Alloc(
   if (id  < 0)
     return NULL;
   MemoryChunk* mc = new MemoryChunk(id, shm, helper_);
+  allocated_memory_ += mc->GetSize();
   chunks_.push_back(mc);
   void* mem = mc->Alloc(size);
   GPU_DCHECK(mem);
@@ -97,6 +125,7 @@ void MappedMemoryManager::FreeUnused() {
     chunk->FreeUnused();
     if (!chunk->InUse()) {
       cmd_buf->DestroyTransferBuffer(chunk->shm_id());
+      allocated_memory_ -= chunk->GetSize();
       iter = chunks_.erase(iter);
     } else {
       ++iter;
@@ -105,6 +134,3 @@ void MappedMemoryManager::FreeUnused() {
 }
 
 }  // namespace gpu
-
-
-