gpu: Reuse transfer buffers more aggresively

gpu/command_buffer/client/fenced_allocator.cc

 // Copyright (c) 2011 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.
 
 // This file contains the implementation of the FencedAllocator class.
 
 #include "gpu/command_buffer/client/fenced_allocator.h"
 
 #include <algorithm>
 
@@ skipping 24 matching lines @@
 
 FencedAllocator::FencedAllocator(unsigned int size,
                                  CommandBufferHelper *helper)
     : helper_(helper),
       bytes_in_use_(0) {
   Block block = { FREE, 0, RoundDown(size), kUnusedToken };
   blocks_.push_back(block);
 }
 
 FencedAllocator::~FencedAllocator() {
-  // Free blocks pending tokens.
+  // Free blocks pending tokens and serials.
   for (unsigned int i = 0; i < blocks_.size(); ++i) {
-    if (blocks_[i].state == FREE_PENDING_TOKEN) {
-      i = WaitForTokenAndFreeBlock(i);
+    switch (blocks_[i].state) {
+      case FREE_PENDING_TOKEN:
+        i = WaitForTokenAndFreeBlock(i);
+        break;
+      case FREE_PENDING_SERIAL:
+        blocks_[i].state = FREE;
+        CollapseFreeBlock(i);

[reveman, 2014/01/16 17:24:49]

I don't think it makes sense free this immediately but wait for a token in the
above case. If this doesn't need to wait for a serial then it at least has to
wait for a token.

I assume that the WaitForToken code above protects against freeing a block
before all commands that might reference it have been processed on the service
side. Why wouldn't we need the same protection for blocks used with async
transfers?

[piman, 2014/01/16 21:22:50]

That is actually not needed to ensure things on the service side, since the
commands that refer to the SHM would execute before the IPC to destroy the SHM
gets handled (assuming proper order of operations).

As I mentioned earlier, If the assumption is that the SHM will be destroyed
after this, then there is no synchronization issue. If the SHM will get reused
with a fresh new FencedAllocator, then we need some sort of synchronization,
yes.
I agree that being consistent is better. However to be able to "finish", we need
to dependency-inject a way to call the right WaitAsyncTexImage2DCHROMIUM that
corresponds to the serial.
An alternative is to document that it's the responsibility of the client to
either destroy the SHM, or ensure synchronization before reuse.

[jadahl, 2014/01/17 08:50:25]

We could just helper_->Finish() before the loop to make sure all potential
tokens have passed; but that wouldn't synchronize the async commands. To do that
now we'd need to busy-wait, hoping the uploader didn't stop uploading.

If we really don't want to put the responsibility to some layer above, could we
put some kind of waitable here and add another flush-serial command that either
signals the waitable when done, or when aborted?

[reveman, 2014/01/17 16:56:47]

If we're adding an async-token, then some command that will wait for the token
on the service side probably makes sense too. We already have a command that
will wait for an async upload to finish so this shouldn't be hard to implement.

+        break;
+      default:
+        break;
     }
   }
   // These checks are not valid if the service has crashed or lost the context.
   // DCHECK_EQ(blocks_.size(), 1u);
   // DCHECK_EQ(blocks_[0].state, FREE);
 }
 
 // Looks for a non-allocated block that is big enough. Search in the FREE
 // blocks first (for direct usage), first-fit, then in the FREE_PENDING_TOKEN
 // blocks, waiting for them. The current implementation isn't smart about
@@ skipping 47 matching lines @@
 void FencedAllocator::FreePendingToken(
     FencedAllocator::Offset offset, int32 token) {
   BlockIndex index = GetBlockByOffset(offset);
   Block &block = blocks_[index];
   if (block.state == IN_USE)
     bytes_in_use_ -= block.size;
   block.state = FREE_PENDING_TOKEN;
   block.token = token;
 }
 
+void FencedAllocator::FreePendingSerial(
+    FencedAllocator::Offset offset, uint32 serial) {
+  BlockIndex index = GetBlockByOffset(offset);
+  Block &block = blocks_[index];
+  if (block.state == IN_USE)
+    bytes_in_use_ -= block.size;
+  block.state = FREE_PENDING_SERIAL;
+  block.serial = serial;
+}
+
 // Gets the max of the size of the blocks marked as free.
 unsigned int FencedAllocator::GetLargestFreeSize() {
   FreeUnused();
   unsigned int max_size = 0;
   for (unsigned int i = 0; i < blocks_.size(); ++i) {
     Block &block = blocks_[i];
     if (block.state == FREE)
       max_size = std::max(max_size, block.size);
   }
   return max_size;
 }
 
 // Gets the size of the largest segment of blocks that are either FREE or
 // FREE_PENDING_TOKEN.
 unsigned int FencedAllocator::GetLargestFreeOrPendingSize() {
   unsigned int max_size = 0;
   unsigned int current_size = 0;
   for (unsigned int i = 0; i < blocks_.size(); ++i) {
     Block &block = blocks_[i];
-    if (block.state == IN_USE) {
-      max_size = std::max(max_size, current_size);
-      current_size = 0;
-    } else {
-      DCHECK(block.state == FREE || block.state == FREE_PENDING_TOKEN);
-      current_size += block.size;
+    switch (block.state) {
+      case IN_USE:
+      case FREE_PENDING_SERIAL:
+        max_size = std::max(max_size, current_size);
+        current_size = 0;
+        break;
+      default:
+        DCHECK(block.state == FREE || block.state == FREE_PENDING_TOKEN);
+        current_size += block.size;
+        break;
     }
   }
   return std::max(max_size, current_size);
 }
 
 // Makes sure that:
 // - there is at least one block.
 // - there are no contiguous FREE blocks (they should have been collapsed).
 // - the successive offsets match the block sizes, and they are in order.
 bool FencedAllocator::CheckConsistency() {
@@ skipping 45 matching lines @@
     BlockIndex index) {
   Block &block = blocks_[index];
   DCHECK_EQ(block.state, FREE_PENDING_TOKEN);
   helper_->WaitForToken(block.token);
   block.state = FREE;
   return CollapseFreeBlock(index);
 }
 
 // Frees any blocks pending a token for which the token has been read.
 void FencedAllocator::FreeUnused() {
-  int32 last_token_read = helper_->last_token_read();
   for (unsigned int i = 0; i < blocks_.size();) {
     Block& block = blocks_[i];
-    if (block.state == FREE_PENDING_TOKEN && block.token <= last_token_read) {
-      block.state = FREE;
-      i = CollapseFreeBlock(i);
-    } else {
-      ++i;
+    switch (block.state) {
+      case FREE_PENDING_TOKEN:
+        if (helper_->HasTokenPassed(block.token)) {
+          block.state = FREE;
+          i = CollapseFreeBlock(i);
+        } else {
+          ++i;
+        }
+        break;
+      case FREE_PENDING_SERIAL:
+        if (helper_->HasSerialPassed(block.serial)) {
+          block.state = FREE;
+          i = CollapseFreeBlock(i);
+        } else {
+          ++i;
+        }
+        break;
+      default:
+        ++i;
+        break;
     }
   }
 }
 
 // If the block is exactly the requested size, simply mark it IN_USE, otherwise
 // split it and mark the first one (of the requested size) IN_USE.
 FencedAllocator::Offset FencedAllocator::AllocInBlock(BlockIndex index,
                                                       unsigned int size) {
   Block &block = blocks_[index];
   DCHECK_GE(block.size, size);
   DCHECK_EQ(block.state, FREE);
   Offset offset = block.offset;
   bytes_in_use_ += size;
   if (block.size == size) {
     block.state = IN_USE;
     return offset;
   }
-  Block newblock = { FREE, offset + size, block.size - size, kUnusedToken};
+  Block newblock = {
+    FREE, offset + size,
+    block.size - size,
+    kUnusedToken,
+    kUnusedSerial
+  };
   block.state = IN_USE;
   block.size = size;
   // this is the last thing being done because it may invalidate block;
   blocks_.insert(blocks_.begin() + index + 1, newblock);
   return offset;
 }
 
 // The blocks are in offset order, so we can do a binary search.
 FencedAllocator::BlockIndex FencedAllocator::GetBlockByOffset(Offset offset) {
-  Block templ = { IN_USE, offset, 0, kUnusedToken };
+  Block templ = {
+    IN_USE,
+    offset,
+    0,
+    kUnusedToken,
+    kUnusedSerial
+  };
   Container::iterator it = std::lower_bound(blocks_.begin(), blocks_.end(),
                                             templ, OffsetCmp());
   DCHECK(it != blocks_.end() && it->offset == offset);
   return it-blocks_.begin();
 }
 
 }  // namespace gpu