Revert "GPU: Grow and shrink transfer buffer"

This reverts commit 65c38f54.

Reason for revert: Causing DCHECK failures in VR instrumentation tests

Original change's description:
> GPU: Grow and shrink transfer buffer
> 
> Automatically grow the transfer buffer when it is full and shrink it
> when the full capacity is not being used. Allow choosing larger and
> smaller sizes than before.
> 
> Before we would only grow it when a single transfer request was larger
> than the whole buffer; requests smaller than the whole buffer would just
> block if the buffer was full. Also we would not ever shrink the buffer
> until someone called Free() manually.
> 
> Bug: 850271, 835353, 828363, 856347
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: If2f2235a1d5297c63663398b37e1e30791347a3e
> Reviewed-on: https://chromium-review.googlesource.com/1105505
> Commit-Queue: James Darpinian <jdarpinian@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572135}

TBR=jdarpinian@chromium.org,sunnyps@chromium.org,piman@chromium.org

Change-Id: I031b30234cdcfbd8a4e48c7ebb6fd23312ec10ed
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 850271, 835353, 828363, 856347, 859952
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1124822Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572277}

gpu/command_buffer/client/client_test_helper.cc

@@ -43,11 +43,10 @@ int32_t FakeCommandBufferServiceBase::GetNextFreeTransferBufferId() {
   return -1;
 }
 
-void FakeCommandBufferServiceBase::SetGetBufferHelper(int transfer_buffer_id,
-                                                      int32_t token) {
+void FakeCommandBufferServiceBase::SetGetBufferHelper(int transfer_buffer_id) {
   ++state_.set_get_buffer_count;
   state_.get_offset = 0;
-  state_.token = token;
+  state_.token = 10000;  // All token checks in the tests should pass.
 }
 
 scoped_refptr<gpu::Buffer>
@@ -132,7 +131,7 @@ CommandBuffer::State MockClientCommandBuffer::WaitForGetOffsetInRange(
 }
 
 void MockClientCommandBuffer::SetGetBuffer(int transfer_buffer_id) {
-  SetGetBufferHelper(transfer_buffer_id, token_);
+  SetGetBufferHelper(transfer_buffer_id);
 }
 
 scoped_refptr<gpu::Buffer> MockClientCommandBuffer::CreateTransferBuffer(

gpu/command_buffer/client/client_test_helper.h

@@ -44,7 +44,7 @@ class FakeCommandBufferServiceBase : public CommandBufferServiceBase {
   int32_t GetNextFreeTransferBufferId();
 
   void FlushHelper(int32_t put_offset);
-  void SetGetBufferHelper(int transfer_buffer_id, int32_t token);
+  void SetGetBufferHelper(int transfer_buffer_id);
   scoped_refptr<gpu::Buffer> CreateTransferBufferHelper(size_t size,
                                                         int32_t* id);
   void DestroyTransferBufferHelper(int32_t id);
@@ -80,11 +80,8 @@ class MockClientCommandBuffer : public CommandBuffer,
 
   int32_t GetServicePutOffset() { return put_offset_; }
 
-  void SetTokenForSetGetBuffer(int32_t token) { token_ = token; }
-
  private:
   int32_t put_offset_ = 0;
-  int32_t token_ = 10000;  // All token checks in the tests should pass.
 };
 
 class MockClientCommandBufferMockFlush : public MockClientCommandBuffer {

gpu/command_buffer/client/gles2_implementation.cc

@@ -1546,12 +1546,12 @@ void GLES2Implementation::DeleteSyncStub(GLsizei n, const GLuint* syncs) {
 GLint GLES2Implementation::GetAttribLocationHelper(
     GLuint program, const char* name) {
   typedef cmds::GetAttribLocation::Result Result;
-  SetBucketAsCString(kResultBucketId, name);
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return -1;
   }
   *result = -1;
+  SetBucketAsCString(kResultBucketId, name);
   helper_->GetAttribLocation(
       program, kResultBucketId, GetResultShmId(), GetResultShmOffset());
   WaitForCmd();
@@ -1575,12 +1575,12 @@ GLint GLES2Implementation::GetAttribLocation(
 GLint GLES2Implementation::GetUniformLocationHelper(
     GLuint program, const char* name) {
   typedef cmds::GetUniformLocation::Result Result;
-  SetBucketAsCString(kResultBucketId, name);
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return -1;
   }
   *result = -1;
+  SetBucketAsCString(kResultBucketId, name);
   helper_->GetUniformLocation(program, kResultBucketId,
                                     GetResultShmId(), GetResultShmOffset());
   WaitForCmd();
@@ -1662,12 +1662,12 @@ bool GLES2Implementation::GetProgramivHelper(
 GLint GLES2Implementation::GetFragDataIndexEXTHelper(GLuint program,
                                                      const char* name) {
   typedef cmds::GetFragDataIndexEXT::Result Result;
-  SetBucketAsCString(kResultBucketId, name);
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return -1;
   }
   *result = -1;
+  SetBucketAsCString(kResultBucketId, name);
   helper_->GetFragDataIndexEXT(program, kResultBucketId, GetResultShmId(),
                                GetResultShmOffset());
   WaitForCmd();
@@ -1691,12 +1691,12 @@ GLint GLES2Implementation::GetFragDataIndexEXT(GLuint program,
 GLint GLES2Implementation::GetFragDataLocationHelper(
     GLuint program, const char* name) {
   typedef cmds::GetFragDataLocation::Result Result;
-  SetBucketAsCString(kResultBucketId, name);
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return -1;
   }
   *result = -1;
+  SetBucketAsCString(kResultBucketId, name);
   helper_->GetFragDataLocation(
       program, kResultBucketId, GetResultShmId(), GetResultShmOffset());
   WaitForCmd();
@@ -1720,12 +1720,12 @@ GLint GLES2Implementation::GetFragDataLocation(
 GLuint GLES2Implementation::GetUniformBlockIndexHelper(
     GLuint program, const char* name) {
   typedef cmds::GetUniformBlockIndex::Result Result;
-  SetBucketAsCString(kResultBucketId, name);
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return GL_INVALID_INDEX;
   }
   *result = GL_INVALID_INDEX;
+  SetBucketAsCString(kResultBucketId, name);
   helper_->GetUniformBlockIndex(
       program, kResultBucketId, GetResultShmId(), GetResultShmOffset());
   WaitForCmd();
@@ -4956,12 +4956,12 @@ GLboolean GLES2Implementation::EnableFeatureCHROMIUM(
                  << feature << ")");
   TRACE_EVENT0("gpu", "GLES2::EnableFeatureCHROMIUM");
   typedef cmds::EnableFeatureCHROMIUM::Result Result;
-  SetBucketAsCString(kResultBucketId, feature);
   Result* result = GetResultAs<Result*>();
   if (!result) {
     return false;
   }
   *result = 0;
+  SetBucketAsCString(kResultBucketId, feature);
   helper_->EnableFeatureCHROMIUM(
       kResultBucketId, GetResultShmId(), GetResultShmOffset());
   WaitForCmd();

gpu/command_buffer/client/implementation_base.cc

@@ -156,7 +156,7 @@ gpu::ContextResult ImplementationBase::Initialize(
   if (!transfer_buffer_->Initialize(
           limits.start_transfer_buffer_size, kStartingOffset,
           limits.min_transfer_buffer_size, limits.max_transfer_buffer_size,
-          kAlignment)) {
+          kAlignment, kSizeToFlush)) {
     // TransferBuffer::Initialize doesn't fail for transient reasons such as if
     // the context was lost. See http://crrev.com/c/720269
     LOG(ERROR) << "ContextResult::kFatalFailure: "

gpu/command_buffer/client/implementation_base.h

@@ -51,6 +51,9 @@ class GLES2_IMPL_EXPORT ImplementationBase
   // used for testing only. If more things are reseved add them here.
   static const unsigned int kStartingOffset = kMaxSizeOfSimpleResult;
 
+  // Size in bytes to issue async flush for transfer buffer.
+  static const unsigned int kSizeToFlush = 256 * 1024;
+
   // Alignment of allocations.
   static const unsigned int kAlignment = 16;
 

gpu/command_buffer/client/mock_transfer_buffer.cc

@@ -44,7 +44,8 @@ bool MockTransferBuffer::Initialize(unsigned int starting_buffer_size,
                                     unsigned int result_size,
                                     unsigned int /* min_buffer_size */,
                                     unsigned int /* max_buffer_size */,
-                                    unsigned int alignment) {
+                                    unsigned int alignment,
+                                    unsigned int /* size_to_flush */) {
   // Just check they match.
   return size_ == starting_buffer_size && result_size_ == result_size &&
          alignment_ == alignment && !initialize_fail_;

gpu/command_buffer/client/mock_transfer_buffer.h

@@ -35,7 +35,8 @@ class MockTransferBuffer : public TransferBufferInterface {
                   unsigned int result_size,
                   unsigned int /* min_buffer_size */,
                   unsigned int /* max_buffer_size */,
-                  unsigned int alignment) override;
+                  unsigned int alignment,
+                  unsigned int size_to_flush) override;
   int GetShmId() override;
   void* GetResultBuffer() override;
   int GetResultOffset() override;

gpu/command_buffer/client/ring_buffer.h

@@ -80,9 +80,6 @@ class GPU_EXPORT RingBuffer {
     return size_ - size_ % alignment_;
   }
 
-  // Total size minus usable size.
-  unsigned int GetUsedSize() { return size_ - GetLargestFreeSizeNoWaiting(); }
-
   // Gets a pointer to a memory block given the base memory and the offset.
   void* GetPointer(RingBuffer::Offset offset) const {
     return static_cast<int8_t*>(base_) + offset;

gpu/command_buffer/client/shared_memory_limits.h

@@ -28,17 +28,17 @@ struct SharedMemoryLimits {
     // On memory constrained devices, switch to lower limits.
     if (base::SysInfo::AmountOfPhysicalMemoryMB() <= 512) {
       command_buffer_size = 512 * 1024;
-      start_transfer_buffer_size = 32 * 1024;
-      min_transfer_buffer_size = 32 * 1024;
+      start_transfer_buffer_size = 256 * 1024;
+      min_transfer_buffer_size = 128 * 1024;
       mapped_memory_chunk_size = 256 * 1024;
     }
 #endif
   }
 
   int32_t command_buffer_size = 1024 * 1024;
-  uint32_t start_transfer_buffer_size = 64 * 1024;
-  uint32_t min_transfer_buffer_size = 64 * 1024;
-  uint32_t max_transfer_buffer_size = 64 * 1024 * 1024;
+  uint32_t start_transfer_buffer_size = 1024 * 1024;
+  uint32_t min_transfer_buffer_size = 256 * 1024;
+  uint32_t max_transfer_buffer_size = 16 * 1024 * 1024;
 
   static constexpr uint32_t kNoLimit = 0;
   uint32_t mapped_memory_reclaim_limit = kNoLimit;

gpu/command_buffer/client/transfer_buffer.cc

@@ -17,17 +17,21 @@
 
 namespace gpu {
 
-TransferBuffer::TransferBuffer(CommandBufferHelper* helper)
+TransferBuffer::TransferBuffer(
+    CommandBufferHelper* helper)
     : helper_(helper),
       result_size_(0),
       default_buffer_size_(0),
       min_buffer_size_(0),
       max_buffer_size_(0),
       alignment_(0),
+      size_to_flush_(0),
+      bytes_since_last_flush_(0),
       buffer_id_(-1),
       result_buffer_(NULL),
       result_shm_offset_(0),
-      usable_(true) {}
+      usable_(true) {
+}
 
 TransferBuffer::~TransferBuffer() {
   Free();
@@ -41,16 +45,19 @@ base::SharedMemoryHandle TransferBuffer::shared_memory_handle() const {
   return buffer_->backing()->shared_memory_handle();
 }
 
-bool TransferBuffer::Initialize(unsigned int default_buffer_size,
+bool TransferBuffer::Initialize(
+    unsigned int default_buffer_size,
     unsigned int result_size,
     unsigned int min_buffer_size,
     unsigned int max_buffer_size,
-                                unsigned int alignment) {
+    unsigned int alignment,
+    unsigned int size_to_flush) {
   result_size_ = result_size;
   default_buffer_size_ = default_buffer_size;
   min_buffer_size_ = min_buffer_size;
   max_buffer_size_ = max_buffer_size;
   alignment_ = alignment;
+  size_to_flush_ = size_to_flush;
   ReallocateRingBuffer(default_buffer_size_ - result_size);
   return HaveBuffer();
 }
@@ -64,10 +71,8 @@ void TransferBuffer::Free() {
     buffer_ = NULL;
     result_buffer_ = NULL;
     result_shm_offset_ = 0;
-    previous_ring_buffers_.push_back(std::move(ring_buffer_));
-    last_allocated_size_ = 0;
-    high_water_mark_ = GetPreviousRingBufferUsedBytes();
-    bytes_since_last_shrink_ = 0;
+    ring_buffer_.reset();
+    bytes_since_last_flush_ = 0;
   }
 }
 
@@ -86,6 +91,10 @@ void TransferBuffer::DiscardBlock(void* p) {
 
 void TransferBuffer::FreePendingToken(void* p, unsigned int token) {
   ring_buffer_->FreePendingToken(p, token);
+  if (bytes_since_last_flush_ >= size_to_flush_ && size_to_flush_ > 0) {
+    helper_->Flush();
+    bytes_since_last_flush_ = 0;
+  }
 }
 
 unsigned int TransferBuffer::GetSize() const {
@@ -110,7 +119,6 @@ void TransferBuffer::AllocateRingBuffer(unsigned int size) {
     scoped_refptr<gpu::Buffer> buffer =
         helper_->command_buffer()->CreateTransferBuffer(size, &id);
     if (id != -1) {
-      last_allocated_size_ = size;
       DCHECK(buffer.get());
       buffer_ = buffer;
       ring_buffer_ = std::make_unique<RingBuffer>(
@@ -119,7 +127,6 @@ void TransferBuffer::AllocateRingBuffer(unsigned int size) {
       buffer_id_ = id;
       result_buffer_ = buffer_->memory();
       result_shm_offset_ = 0;
-      bytes_since_last_shrink_ = 0;
       return;
     }
     // we failed so don't try larger than this.
@@ -132,7 +139,7 @@ static unsigned int ComputePOTSize(unsigned int dimension) {
   return (dimension == 0) ? 0 : 1 << base::bits::Log2Ceiling(dimension);
 }
 
-void TransferBuffer::ReallocateRingBuffer(unsigned int size, bool shrink) {
+void TransferBuffer::ReallocateRingBuffer(unsigned int size) {
   // What size buffer would we ask for if we needed a new one?
   unsigned int needed_buffer_size = ComputePOTSize(size + result_size_);
   DCHECK_EQ(needed_buffer_size % alignment_, 0u)
@@ -141,11 +148,7 @@ void TransferBuffer::ReallocateRingBuffer(unsigned int size, bool shrink) {
   needed_buffer_size = std::max(needed_buffer_size, default_buffer_size_);
   needed_buffer_size = std::min(needed_buffer_size, max_buffer_size_);
 
-  unsigned int current_size = HaveBuffer() ? buffer_->size() : 0;
-  if (current_size == needed_buffer_size)
-    return;
-
-  if (usable_ && (shrink || needed_buffer_size > current_size)) {
+  if (usable_ && (!HaveBuffer() || needed_buffer_size > buffer_->size())) {
     if (HaveBuffer()) {
       Free();
     }
@@ -153,49 +156,11 @@ void TransferBuffer::ReallocateRingBuffer(unsigned int size, bool shrink) {
   }
 }
 
-unsigned int TransferBuffer::GetPreviousRingBufferUsedBytes() {
-  while (!previous_ring_buffers_.empty() &&
-         previous_ring_buffers_.front()->GetUsedSize() == 0) {
-    previous_ring_buffers_.pop_front();
-  }
-  unsigned int total = 0;
-  for (auto& buffer : previous_ring_buffers_) {
-    total += buffer->GetUsedSize();
-  }
-  return total;
-}
-
-void TransferBuffer::ShrinkOrExpandRingBufferIfNecessary(
-    unsigned int size_to_allocate) {
-  unsigned int available_size = GetFreeSize();
-  high_water_mark_ =
-      std::max(high_water_mark_, last_allocated_size_ - available_size +
-                                     size_to_allocate +
-                                     GetPreviousRingBufferUsedBytes());
-  if (size_to_allocate > available_size) {
-    // Try to expand the ring buffer.
-    ReallocateRingBuffer(high_water_mark_);
-  } else if (bytes_since_last_shrink_ > high_water_mark_ * kShrinkThreshold) {
-    // The intent of the above check is to limit the frequency of buffer shrink
-    // attempts. Unfortunately if an application uploads a large amount of data
-    // once and from then on uploads only a small amount per frame, it will be a
-    // very long time before we attempt to shrink (or forever, if no data is
-    // uploaded).
-    // TODO(jdarpinian): Change this heuristic to be based on frame number
-    // instead, and consider shrinking at the end of each frame (for clients
-    // that have a notion of frames).
-    bytes_since_last_shrink_ = 0;
-    ReallocateRingBuffer(high_water_mark_ + high_water_mark_ / 4,
-                         true /* shrink */);
-    high_water_mark_ = size_to_allocate + GetPreviousRingBufferUsedBytes();
-  }
-}
-
 void* TransferBuffer::AllocUpTo(
     unsigned int size, unsigned int* size_allocated) {
   DCHECK(size_allocated);
 
-  ShrinkOrExpandRingBufferIfNecessary(size);
+  ReallocateRingBuffer(size);
 
   if (!HaveBuffer()) {
     return NULL;
@@ -203,12 +168,12 @@ void* TransferBuffer::AllocUpTo(
 
   unsigned int max_size = ring_buffer_->GetLargestFreeOrPendingSize();
   *size_allocated = std::min(max_size, size);
-  bytes_since_last_shrink_ += *size_allocated;
+  bytes_since_last_flush_ += *size_allocated;
   return ring_buffer_->Alloc(*size_allocated);
 }
 
 void* TransferBuffer::Alloc(unsigned int size) {
-  ShrinkOrExpandRingBufferIfNecessary(size);
+  ReallocateRingBuffer(size);
 
   if (!HaveBuffer()) {
     return NULL;
@@ -218,7 +183,8 @@ void* TransferBuffer::Alloc(unsigned int size) {
   if (size > max_size) {
     return NULL;
   }
-  bytes_since_last_shrink_ += size;
+
+  bytes_since_last_flush_ += size;
   return ring_buffer_->Alloc(size);
 }
 

gpu/command_buffer/client/transfer_buffer.h

@@ -11,7 +11,6 @@
 #include <memory>
 
 #include "base/compiler_specific.h"
-#include "base/containers/circular_deque.h"
 #include "base/macros.h"
 #include "gpu/command_buffer/client/ring_buffer.h"
 #include "gpu/command_buffer/common/buffer.h"
@@ -37,11 +36,13 @@ class GPU_EXPORT TransferBufferInterface {
   // Otherwise, this returns an invalid handle.
   virtual base::SharedMemoryHandle shared_memory_handle() const = 0;
 
-  virtual bool Initialize(unsigned int buffer_size,
+  virtual bool Initialize(
+      unsigned int buffer_size,
       unsigned int result_size,
       unsigned int min_buffer_size,
       unsigned int max_buffer_size,
-                          unsigned int alignment) = 0;
+      unsigned int alignment,
+      unsigned int size_to_flush) = 0;
 
   virtual int GetShmId() = 0;
   virtual void* GetResultBuffer() = 0;
@@ -85,7 +86,8 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface {
                   unsigned int result_size,
                   unsigned int min_buffer_size,
                   unsigned int max_buffer_size,
-                  unsigned int alignment) override;
+                  unsigned int alignment,
+                  unsigned int size_to_flush) override;
   int GetShmId() override;
   void* GetResultBuffer() override;
   int GetResultOffset() override;
@@ -105,25 +107,14 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface {
   unsigned int GetCurrentMaxAllocationWithoutRealloc() const;
   unsigned int GetMaxAllocation() const;
 
-  // We will attempt to shrink the ring buffer once the number of bytes
-  // allocated reaches this threshold times the high water mark.
-  static const int kShrinkThreshold = 120;
-
  private:
   // Tries to reallocate the ring buffer if it's not large enough for size.
-  void ReallocateRingBuffer(unsigned int size, bool shrink = false);
+  void ReallocateRingBuffer(unsigned int size);
 
   void AllocateRingBuffer(unsigned int size);
 
-  void ShrinkOrExpandRingBufferIfNecessary(unsigned int size);
-
-  // Returns the number of bytes that are still in use in ring buffers that we
-  // previously freed.
-  unsigned int GetPreviousRingBufferUsedBytes();
-
   CommandBufferHelper* helper_;
   std::unique_ptr<RingBuffer> ring_buffer_;
-  base::circular_deque<std::unique_ptr<RingBuffer>> previous_ring_buffers_;
 
   // size reserved for results
   unsigned int result_size_;
@@ -137,17 +128,14 @@ class GPU_EXPORT TransferBuffer : public TransferBufferInterface {
   // max size we'll let the buffer grow
   unsigned int max_buffer_size_;
 
-  // Size of the currently allocated ring buffer.
-  unsigned int last_allocated_size_ = 0;
-
-  // The size to shrink the ring buffer to next time shrinking happens.
-  unsigned int high_water_mark_ = 0;
-
   // alignment for allocations
   unsigned int alignment_;
 
-  // Number of bytes since we last attempted to shrink the ring buffer.
-  unsigned int bytes_since_last_shrink_ = 0;
+  // Size at which to do an async flush. 0 = never.
+  unsigned int size_to_flush_;
+
+  // Number of bytes since we last flushed.
+  unsigned int bytes_since_last_flush_;
 
   // the current buffer.
   scoped_refptr<gpu::Buffer> buffer_;