gpu: Reuse transfer buffers more aggresively

gpu/command_buffer/client/gles2_implementation.cc

Index: gpu/command_buffer/client/gles2_implementation.cc
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 2d02579b116672e26da92edc5e66c7dcc6ae56b8..0fe448231f449acb116871231b43c3449098c6df 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -1378,11 +1378,8 @@ void GLES2Implementation::BufferDataHelper(
 
     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());
-
-      // Remove old buffer.
-      buffer_tracker_->RemoveBuffer(buffer_id);
+      FreeTransferBuffer(buffer);
+      buffer_tracker_->RemoveBuffer(buffer->id());
     }
 
     // Create new buffer.
@@ -1511,6 +1508,23 @@ void GLES2Implementation::BufferSubData(
   CheckGLError();
 }
 
+void GLES2Implementation::FreeTransferBuffer(BufferTracker::Buffer* buffer) {
+  int32 token = buffer->last_usage_token();
+  uint32 async_token = buffer->async_token();
+
+  if (async_token) {
+    if (helper_->HasAsyncTokenPassed(async_token))
+      buffer_tracker_->Free(buffer);
+    else
+      buffer_tracker_->FreePendingAsyncToken(buffer, async_token);
+  } else if (token) {
+    if (helper_->HasTokenPassed(token))
+      buffer_tracker_->Free(buffer);
+    else
+      buffer_tracker_->FreePendingToken(buffer, token);
+  }
+}
+
 bool GLES2Implementation::GetBoundPixelTransferBuffer(
     GLenum target,
     const char* function_name,
@@ -1586,7 +1600,7 @@ void GLES2Implementation::CompressedTexImage2D(
       helper_->CompressedTexImage2D(
           target, level, internalformat, width, height, border, image_size,
           buffer->shm_id(), buffer->shm_offset() + offset);
-      buffer->set_transfer_ready_token(helper_->InsertToken());
+      MarkPixelTransferBufferLastUsage(buffer);
     }
     return;
   }
@@ -1627,7 +1641,7 @@ void GLES2Implementation::CompressedTexSubImage2D(
       helper_->CompressedTexSubImage2D(
           target, level, xoffset, yoffset, width, height, format, image_size,
           buffer->shm_id(), buffer->shm_offset() + offset);
-      buffer->set_transfer_ready_token(helper_->InsertToken());
+      MarkPixelTransferBufferLastUsage(buffer);
       CheckGLError();
     }
     return;
@@ -1714,7 +1728,7 @@ void GLES2Implementation::TexImage2D(
       helper_->TexImage2D(
           target, level, internalformat, width, height, border, format, type,
           buffer->shm_id(), buffer->shm_offset() + offset);
-      buffer->set_transfer_ready_token(helper_->InsertToken());
+      MarkPixelTransferBufferLastUsage(buffer);
       CheckGLError();
     }
     return;
@@ -1820,7 +1834,7 @@ void GLES2Implementation::TexSubImage2D(
       helper_->TexSubImage2D(
           target, level, xoffset, yoffset, width, height, format, type,
           buffer->shm_id(), buffer->shm_offset() + offset, false);
-      buffer->set_transfer_ready_token(helper_->InsertToken());
+      MarkPixelTransferBufferLastUsage(buffer);
       CheckGLError();
     }
     return;
@@ -2403,24 +2417,24 @@ void GLES2Implementation::GenQueriesEXTHelper(
 // deleted the resource.
 
 bool GLES2Implementation::BindBufferHelper(
-    GLenum target, GLuint buffer) {
+    GLenum target, GLuint buffer_id) {
   // 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;
+      bound_pixel_unpack_transfer_buffer_id_ = buffer_id;
       break;
     default:
       changed = true;
@@ -2428,7 +2442,7 @@ bool GLES2Implementation::BindBufferHelper(
   }
   // 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;
 }
 
@@ -2562,13 +2576,13 @@ void GLES2Implementation::DeleteBuffersHelper(
       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]);
+      FreeTransferBuffer(buffer);
+      buffer_tracker_->RemoveBuffer(buffer->id());
     }
+
     if (buffers[ii] == bound_pixel_unpack_transfer_buffer_id_) {
       bound_pixel_unpack_transfer_buffer_id_ = 0;
     }
@@ -3611,9 +3625,9 @@ void* GLES2Implementation::MapBufferCHROMIUM(GLuint target, GLenum access) {
   // with this method of synchronization. Until this is fixed,
   // MapBufferCHROMIUM will not block even if the transfer is not ready
   // for these calls.
-  if (buffer->transfer_ready_token()) {
-    helper_->WaitForToken(buffer->transfer_ready_token());
-    buffer->set_transfer_ready_token(0);
+  if (buffer->last_usage_token()) {
+    helper_->WaitForToken(buffer->last_usage_token());
+    buffer->set_last_usage_token(0);
   }
   buffer->set_mapped(true);
 
@@ -3647,6 +3661,16 @@ GLboolean GLES2Implementation::UnmapBufferCHROMIUM(GLuint target) {
   return true;
 }
 
+void GLES2Implementation::InsertAsyncPixelUnpackToken(
+    BufferTracker::Buffer *buffer) {
+  buffer->set_async_token(helper_->InsertAsyncToken());
+}
+
+void GLES2Implementation::MarkPixelTransferBufferLastUsage(
+    BufferTracker::Buffer* buffer) {
+  buffer->set_last_usage_token(helper_->InsertToken());
+}

[reveman, 2014/01/22 17:30:04]

IMO, the code is easier to understand without these helper functions. Especially
if s/set_async_token/set_last_async_usage_token/

+
 void GLES2Implementation::AsyncTexImage2DCHROMIUM(
     GLenum target, GLint level, GLint internalformat, GLsizei width,
     GLsizei height, GLint border, GLenum format, GLenum type,
@@ -3694,6 +3718,7 @@ void GLES2Implementation::AsyncTexImage2DCHROMIUM(
     helper_->AsyncTexImage2DCHROMIUM(
         target, level, internalformat, width, height, border, format, type,
         buffer->shm_id(), buffer->shm_offset() + offset);
+    InsertAsyncPixelUnpackToken(buffer);
   }
 }
 
@@ -3738,6 +3763,7 @@ void GLES2Implementation::AsyncTexSubImage2DCHROMIUM(
     helper_->AsyncTexSubImage2DCHROMIUM(
         target, level, xoffset, yoffset, width, height, format, type,
         buffer->shm_id(), buffer->shm_offset() + offset);
+    InsertAsyncPixelUnpackToken(buffer);
   }
 }