gpu: Reuse transfer buffers more aggresively

gpu/command_buffer/client/gles2_implementation.cc

 // Copyright (c) 2012 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.
 
 // A class to emulate GLES2 over command buffers.
 
 #include "gpu/command_buffer/client/gles2_implementation.h"
 
 #include <algorithm>
 #include <map>
@@ skipping 1361 matching lines @@
   }
 
   GLuint buffer_id;
   if (GetBoundPixelTransferBuffer(target, "glBufferData", &buffer_id)) {
     if (!buffer_id) {
       return;
     }
 
     BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffer_id);
     if (buffer) {
-      // Free buffer memory, pending the passage of a token.
-      buffer_tracker_->FreePendingToken(buffer, helper_->InsertToken());
+      if (buffer->used()) {
+        buffer->set_used(false);
+        buffer->set_unused_token(helper_->InsertToken());
+      }
+      DisposeTransferBuffer(buffer);
+    }
 
-      // Remove old buffer.
-      buffer_tracker_->RemoveBuffer(buffer_id);
-    }
+    CheckBuffersPendingAsyncComplete();
 
     // Create new buffer.
     buffer = buffer_tracker_->CreateBuffer(buffer_id, size);
     DCHECK(buffer);
+    buffer->set_used(true);
     if (buffer->address() && data)
       memcpy(buffer->address(), data, size);
     return;
   }
 
   if (size == 0) {
     return;
   }
 
   // If there is no data just send BufferData
@@ skipping 103 matching lines @@
     GLenum target, GLintptr offset, GLsizeiptr size, const void* data) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glBufferSubData("
       << GLES2Util::GetStringBufferTarget(target) << ", "
       << offset << ", " << size << ", "
       << static_cast<const void*>(data) << ")");
   BufferSubDataHelper(target, offset, size, data);
   CheckGLError();
 }
 
+void GLES2Implementation::FreeTransferBuffer(BufferTracker::Buffer *buffer) {
+  int32 token = buffer->unused_token();
+  // FIXME: This kind of checks exist elsewhere; is it correct?
+  if (token) {
+    if (helper_->HasTokenPassed(token))

[piman, 2014/01/08 05:08:04]

Maybe this logic (if the token is passed, free immediately) makes sense as low
as possible, maybe FencedAllocator. That avoids some duplication of logic, and
might also hit some other cases.

+      buffer_tracker_->Free(buffer);
+    else
+      buffer_tracker_->FreePendingToken(buffer, token);
+  }
+}
+
+void GLES2Implementation::DisposeTransferBuffer(BufferTracker::Buffer* buffer) {
+  if (buffer->async_query_id()) {
+    QueryTracker::Query* query =
+        query_tracker_->GetQuery(buffer->async_query_id());
+    if (!query->CheckResultsAvailable(helper_)) {
+      // Async upload not completed, queue release.
+      released_buffers_pending_async_complete_.push_back(buffer);
+      buffer_tracker_->UnmanageBuffer(buffer->id());
+      return;
+    }
+  }
+
+  FreeTransferBuffer(buffer);
+  buffer_tracker_->RemoveBuffer(buffer->id());
+}
+
+void GLES2Implementation::CheckBuffersPendingAsyncComplete() {

[epennerAtGoogle, 2014/01/07 00:47:59]

Hmm, it's unfortunate that we have to check all of these every time we create a
buffer.

Uploads are guaranteed to complete in order, but I guess we can't use that
assumption to break out of the loop, since there is no guarantee they are
deleted in order...  I'm not sure how to solve this yet but I'm hesitant of
adding a loop over all pending buffers.

[reveman, 2014/01/07 06:32:06]

you could just keep the list sorted based on query age, which would be the case
if we follow the suggestion I made earlier about enumerating queries and handle
the release of buffers in the mapped memory manager consistent with how we free
based on passed tokens.

[jadahl, 2014/01/07 10:52:03]

The enumeration will improve this situation indeed.

+  BufferList::iterator it = released_buffers_pending_async_complete_.begin();
+  while (it != released_buffers_pending_async_complete_.end()) {
+    BufferTracker::Buffer* buffer = *it;
+    QueryTracker::Query* query =
+        query_tracker_->GetQuery(buffer->async_query_id());
+    if (query->CheckResultsAvailable(helper_)) {
+      it = released_buffers_pending_async_complete_.erase(it);
+      FreeTransferBuffer(buffer);
+      delete buffer;
+    } else {
+      ++it;
+    }
+  }
+}
+
 bool GLES2Implementation::GetBoundPixelTransferBuffer(
     GLenum target,
     const char* function_name,
     GLuint* buffer_id) {
   *buffer_id = 0;
 
   switch (target) {
     case GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM:
       *buffer_id = bound_pixel_pack_transfer_buffer_id_;
       break;
@@ skipping 872 matching lines @@
 }
 
 // NOTE #1: On old versions of OpenGL, calling glBindXXX with an unused id
 // generates a new resource. On newer versions of OpenGL they don't. The code
 // related to binding below will need to change if we switch to the new OpenGL
 // model. Specifically it assumes a bind will succeed which is always true in
 // the old model but possibly not true in the new model if another context has
 // deleted the resource.
 
 bool GLES2Implementation::BindBufferHelper(
-    GLenum target, GLuint buffer) {
+    GLenum target, GLuint buffer_id) {
+  BufferTracker::Buffer* buffer;
+
   // TODO(gman): See note #1 above.
   bool changed = false;
   switch (target) {
     case GL_ARRAY_BUFFER:
-      if (bound_array_buffer_id_ != buffer) {
-        bound_array_buffer_id_ = buffer;
+      if (bound_array_buffer_id_ != buffer_id) {
+        bound_array_buffer_id_ = buffer_id;
         changed = true;
       }
       break;
     case GL_ELEMENT_ARRAY_BUFFER:
-      changed = vertex_array_object_manager_->BindElementArray(buffer);
+      changed = vertex_array_object_manager_->BindElementArray(buffer_id);
       break;
     case GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM:
-      bound_pixel_pack_transfer_buffer_id_ = buffer;
+      bound_pixel_pack_transfer_buffer_id_ = buffer_id;
       break;
     case GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM:
-      bound_pixel_unpack_transfer_buffer_id_ = buffer;
+      // Mark any previously bound buffer as unused.
+      if (bound_pixel_unpack_transfer_buffer_id_ != buffer_id &&
+          bound_pixel_unpack_transfer_buffer_id_ != 0) {
+        buffer =
+            buffer_tracker_->GetBuffer(bound_pixel_unpack_transfer_buffer_id_);
+        if (buffer) {
+          buffer->set_unused_token(helper_->InsertToken());
+        }
+      }
+
+      bound_pixel_unpack_transfer_buffer_id_ = buffer_id;
+      buffer = buffer_tracker_->GetBuffer(buffer_id);
+      if (buffer) {
+        buffer->set_used(true);
+        buffer->set_unused_token(0);
+      }
+
       break;
     default:
       changed = true;
       break;
   }
   // TODO(gman): There's a bug here. If the target is invalid the ID will not be
   // used even though it's marked it as used here.
-  GetIdHandler(id_namespaces::kBuffers)->MarkAsUsedForBind(buffer);
+  GetIdHandler(id_namespaces::kBuffers)->MarkAsUsedForBind(buffer_id);
   return changed;
 }
 
 bool GLES2Implementation::BindFramebufferHelper(
     GLenum target, GLuint framebuffer) {
   // TODO(gman): See note #1 above.
   bool changed = false;
   switch (target) {
     case GL_FRAMEBUFFER:
       if (bound_framebuffer_ != framebuffer ||
@@ skipping 113 matching lines @@
     SetGLError(
         GL_INVALID_VALUE,
         "glDeleteBuffers", "id not created by this context.");
     return;
   }
   for (GLsizei ii = 0; ii < n; ++ii) {
     if (buffers[ii] == bound_array_buffer_id_) {
       bound_array_buffer_id_ = 0;
     }
     vertex_array_object_manager_->UnbindBuffer(buffers[ii]);
+
     BufferTracker::Buffer* buffer = buffer_tracker_->GetBuffer(buffers[ii]);
     if (buffer) {
-      // Free buffer memory, pending the passage of a token.
-      buffer_tracker_->FreePendingToken(buffer, helper_->InsertToken());
-      // Remove buffer.
-      buffer_tracker_->RemoveBuffer(buffers[ii]);
+      buffer->set_used(false);
+      buffer->set_unused_token(helper_->InsertToken());
+      DisposeTransferBuffer(buffer);
     }
+
     if (buffers[ii] == bound_pixel_unpack_transfer_buffer_id_) {
       bound_pixel_unpack_transfer_buffer_id_ = 0;
     }
   }
 }
 
 void GLES2Implementation::DeleteBuffersStub(
     GLsizei n, const GLuint* buffers) {
   helper_->DeleteBuffersImmediate(n, buffers);
 }
@@ skipping 1055 matching lines @@
   }
   if (!buffer->mapped()) {
     SetGLError(GL_INVALID_OPERATION, "glUnmapBufferCHROMIUM", "not mapped");
     return false;
   }
   buffer->set_mapped(false);
   CheckGLError();
   return true;
 }
 
+void GLES2Implementation::BeginQueryAsyncPixelUnpackCompleted(
+    BufferTracker::Buffer *buffer) {
+  DCHECK(!current_private_unpack_query_);
+
+  GLuint id;
+  GetIdHandler(id_namespaces::kQueries)->MakeIds(this, 0, 1, &id);
+  QueryTracker::Query* query =
+      query_tracker_->CreateQuery(
+          id, GL_ASYNC_PIXEL_UNPACK_COMPLETED_PRIVATE_CHROMIUM);
+  if (!query) {
+    MustBeContextLost();
+    return;
+  }
+
+  buffer->set_async_query_id(id);
+  query->Begin(this);
+  CheckGLError();
+
+  current_private_unpack_query_ = query;
+}
+
+void GLES2Implementation::EndQueryAsyncPixelUnpackCompleted() {
+  DCHECK(current_private_unpack_query_);
+
+  current_private_unpack_query_->End(this);
+  current_private_unpack_query_ = NULL;
+  CheckGLError();
+}
+
 void GLES2Implementation::AsyncTexImage2DCHROMIUM(
     GLenum target, GLint level, GLint internalformat, GLsizei width,
     GLsizei height, GLint border, GLenum format, GLenum type,
     const void* pixels) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glTexImage2D("
       << GLES2Util::GetStringTextureTarget(target) << ", "
       << level << ", "
       << GLES2Util::GetStringTextureInternalFormat(internalformat) << ", "
       << width << ", " << height << ", " << border << ", "
@@ skipping 24 matching lines @@
 
   // Otherwise, async uploads require a transfer buffer to be bound.
   // TODO(hubbe): Make MapBufferCHROMIUM block if someone tries to re-use
   // the buffer before the transfer is finished. (Currently such
   // synchronization has to be handled manually.)
   GLuint offset = ToGLuint(pixels);
   BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
       bound_pixel_unpack_transfer_buffer_id_,
       "glAsyncTexImage2DCHROMIUM", offset, size);
   if (buffer && buffer->shm_id() != -1) {
+    BeginQueryAsyncPixelUnpackCompleted(buffer);
     helper_->AsyncTexImage2DCHROMIUM(
         target, level, internalformat, width, height, border, format, type,
         buffer->shm_id(), buffer->shm_offset() + offset);
+    EndQueryAsyncPixelUnpackCompleted();
   }
 }
 
 void GLES2Implementation::AsyncTexSubImage2DCHROMIUM(
     GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width,
     GLsizei height, GLenum format, GLenum type, const void* pixels) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glAsyncTexSubImage2DCHROMIUM("
       << GLES2Util::GetStringTextureTarget(target) << ", "
       << level << ", "
@@ skipping 21 matching lines @@
 
   // Async uploads require a transfer buffer to be bound.
   // TODO(hubbe): Make MapBufferCHROMIUM block if someone tries to re-use
   // the buffer before the transfer is finished. (Currently such
   // synchronization has to be handled manually.)
   GLuint offset = ToGLuint(pixels);
   BufferTracker::Buffer* buffer = GetBoundPixelUnpackTransferBufferIfValid(
       bound_pixel_unpack_transfer_buffer_id_,
       "glAsyncTexSubImage2DCHROMIUM", offset, size);
   if (buffer && buffer->shm_id() != -1) {
+    BeginQueryAsyncPixelUnpackCompleted(buffer);
     helper_->AsyncTexSubImage2DCHROMIUM(
         target, level, xoffset, yoffset, width, height, format, type,
         buffer->shm_id(), buffer->shm_offset() + offset);
+    EndQueryAsyncPixelUnpackCompleted();
   }
 }
 
 void GLES2Implementation::WaitAsyncTexImage2DCHROMIUM(GLenum target) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glWaitAsyncTexImage2DCHROMIUM("
       << GLES2Util::GetStringTextureTarget(target) << ")");
   helper_->WaitAsyncTexImage2DCHROMIUM(target);
   CheckGLError();
 }
@@ skipping 166 matching lines @@
   CheckGLError();
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "gpu/command_buffer/client/gles2_implementation_impl_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu