oop: Convert trasfer cache client api to be bulk unlock.

This patch fixes a TODO, and introduces a bulk API for unlocking
transfer cache entries.

R=ericrk@chromium.org, piman@chromium.org

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ic13611fa2763fc7bb20000519f2f4e893264708a
Reviewed-on: https://chromium-review.googlesource.com/828062Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524297}

cc/paint/transfer_cache_serialize_helper.cc

@@ -35,7 +35,8 @@ void TransferCacheSerializeHelper::CreateEntry(
 }
 
 void TransferCacheSerializeHelper::FlushEntries() {
-  FlushEntriesInternal(added_entries_);
+  FlushEntriesInternal(
+      std::vector<EntryKey>(added_entries_.begin(), added_entries_.end()));
   added_entries_.clear();
 }
 

cc/paint/transfer_cache_serialize_helper.h

@@ -6,6 +6,7 @@
 #define CC_PAINT_TRANSFER_CACHE_SERIALIZE_HELPER_H_
 
 #include <set>
+#include <vector>
 
 #include "cc/paint/paint_export.h"
 #include "cc/paint/transfer_cache_entry.h"
@@ -27,7 +28,7 @@ class CC_PAINT_EXPORT TransferCacheSerializeHelper {
   using EntryKey = std::pair<TransferCacheEntryType, uint32_t>;
   virtual bool LockEntryInternal(TransferCacheEntryType type, uint32_t id) = 0;
   virtual void CreateEntryInternal(const ClientTransferCacheEntry& entry) = 0;
-  virtual void FlushEntriesInternal(const std::set<EntryKey>& entries) = 0;
+  virtual void FlushEntriesInternal(const std::vector<EntryKey>& entries) = 0;
 
   std::set<EntryKey> added_entries_;
 };

cc/paint/transfer_cache_unittest.cc

@@ -103,7 +103,7 @@ TEST_F(TransferCacheTest, Basic) {
   EXPECT_NE(nullptr, service_cache->GetEntry(entry.Type(), entry.Id()));
 
   // Unlock on client side and flush to service.
-  context_support->UnlockTransferCacheEntry(entry.Type(), entry.Id());
+  context_support->UnlockTransferCacheEntries({{entry.Type(), entry.Id()}});
   gl->Finish();
 
   // Re-lock on client side and validate state. No need to flush as lock is
@@ -131,7 +131,7 @@ TEST_F(TransferCacheTest, Eviction) {
   EXPECT_NE(nullptr, service_cache->GetEntry(entry.Type(), entry.Id()));
 
   // Unlock on client side and flush to service.
-  context_support->UnlockTransferCacheEntry(entry.Type(), entry.Id());
+  context_support->UnlockTransferCacheEntries({{entry.Type(), entry.Id()}});
   gl->Finish();
 
   // Evict on the service side.

cc/test/test_context_support.cc

@@ -123,9 +123,8 @@ bool TestContextSupport::ThreadsafeLockTransferCacheEntry(
   NOTIMPLEMENTED();
   return false;
 }
-void TestContextSupport::UnlockTransferCacheEntry(
-    TransferCacheEntryType entry_type,
-    uint32_t entry_id) {
+void TestContextSupport::UnlockTransferCacheEntries(
+    const std::vector<std::pair<TransferCacheEntryType, uint32_t>>& entries) {
   NOTIMPLEMENTED();
 }
 void TestContextSupport::DeleteTransferCacheEntry(

cc/test/test_context_support.h

@@ -58,8 +58,9 @@ class TestContextSupport : public gpu::ContextSupport {
   void CreateTransferCacheEntry(const ClientTransferCacheEntry& entry) override;
   bool ThreadsafeLockTransferCacheEntry(TransferCacheEntryType entry_type,
                                         uint32_t entry_id) override;
-  void UnlockTransferCacheEntry(TransferCacheEntryType entry_type,
-                                uint32_t entry_id) override;
+  void UnlockTransferCacheEntries(
+      const std::vector<std::pair<TransferCacheEntryType, uint32_t>>& entries)
+      override;
   void DeleteTransferCacheEntry(TransferCacheEntryType entry_type,
                                 uint32_t entry_id) override;
 

cc/test/transfer_cache_test_helper.cc

@@ -57,7 +57,7 @@ void TransferCacheTestHelper::CreateEntryInternal(
 }
 
 void TransferCacheTestHelper::FlushEntriesInternal(
-    const std::set<EntryKey>& entries) {
+    const std::vector<EntryKey>& entries) {
   for (auto& entry : entries)
     locked_entries_.erase(entry);
 }

cc/test/transfer_cache_test_helper.h

@@ -6,7 +6,7 @@
 #define CC_TEST_TRANSFER_CACHE_TEST_HELPER_H_
 
 #include <map>
-#include <set>
+#include <vector>
 
 #include "cc/paint/transfer_cache_deserialize_helper.h"
 #include "cc/paint/transfer_cache_serialize_helper.h"
@@ -31,7 +31,7 @@ class TransferCacheTestHelper : public TransferCacheDeserializeHelper,
   // Serialization helpers.
   bool LockEntryInternal(TransferCacheEntryType type, uint32_t id) override;
   void CreateEntryInternal(const ClientTransferCacheEntry& entry) override;
-  void FlushEntriesInternal(const std::set<EntryKey>& entries) override;
+  void FlushEntriesInternal(const std::vector<EntryKey>& entries) override;
 
   std::map<EntryKey, std::unique_ptr<ServiceTransferCacheEntry>> entries_;
   std::set<EntryKey> locked_entries_;

gpu/command_buffer/client/client_transfer_cache.cc

@@ -53,13 +53,17 @@ bool ClientTransferCache::LockTransferCacheEntry(
   return false;
 }
 
-void ClientTransferCache::UnlockTransferCacheEntry(
+void ClientTransferCache::UnlockTransferCacheEntries(
     gles2::GLES2CmdHelper* helper,
-    cc::TransferCacheEntryType type,
-    uint32_t id) {
+    const std::vector<std::pair<cc::TransferCacheEntryType, uint32_t>>&
+        entries) {
   base::AutoLock hold(lock_);
+  for (const auto& entry : entries) {
+    auto type = entry.first;
+    auto id = entry.second;
     DCHECK(!FindDiscardableHandleId(type, id).is_null());
     helper->UnlockTransferCacheEntryINTERNAL(static_cast<uint32_t>(type), id);
+  }
 }
 
 void ClientTransferCache::DeleteTransferCacheEntry(

gpu/command_buffer/client/client_transfer_cache.h

@@ -24,7 +24,7 @@ class MappedMemoryManager;
 // them available for consumption in the GPU process. Typical usage is:
 //   1) Insert a new entry via CreateCacheEntry. It starts locked.
 //   2) Use the new entry's ID in one or more commands.
-//   3) Unlock the entry via UnlockTransferCacheEntry after dependent commands
+//   3) Unlock the entry via UnlockTransferCacheEntries after dependent commands
 //      have been issued.
 //
 // If an entry is needed again:
@@ -48,9 +48,10 @@ class GLES2_IMPL_EXPORT ClientTransferCache {
                         MappedMemoryManager* mapped_memory,
                         const cc::ClientTransferCacheEntry& entry);
   bool LockTransferCacheEntry(cc::TransferCacheEntryType type, uint32_t id);
-  void UnlockTransferCacheEntry(gles2::GLES2CmdHelper* helper,
-                                cc::TransferCacheEntryType type,
-                                uint32_t id);
+  void UnlockTransferCacheEntries(
+      gles2::GLES2CmdHelper* helper,
+      const std::vector<std::pair<cc::TransferCacheEntryType, uint32_t>>&
+          entries);
   void DeleteTransferCacheEntry(gles2::GLES2CmdHelper* helper,
                                 cc::TransferCacheEntryType type,
                                 uint32_t id);

gpu/command_buffer/client/context_support.h

@@ -102,8 +102,9 @@ class ContextSupport {
       const cc::ClientTransferCacheEntry& entry) = 0;
   virtual bool ThreadsafeLockTransferCacheEntry(cc::TransferCacheEntryType type,
                                                 uint32_t id) = 0;
-  virtual void UnlockTransferCacheEntry(cc::TransferCacheEntryType type,
-                                        uint32_t id) = 0;
+  virtual void UnlockTransferCacheEntries(
+      const std::vector<std::pair<cc::TransferCacheEntryType, uint32_t>>&
+          entries) = 0;
   virtual void DeleteTransferCacheEntry(cc::TransferCacheEntryType type,
                                         uint32_t id) = 0;
 

gpu/command_buffer/client/gles2_implementation.cc

@@ -119,11 +119,9 @@ class TransferCacheSerializeHelperImpl
   }
 
   void FlushEntriesInternal(
-      const std::set<std::pair<cc::TransferCacheEntryType, uint32_t>>& entries)
-      final {
-    // TODO(vmpstr): Add a bulk unlock API instead.
-    for (auto& entry : entries)
-      gl_->UnlockTransferCacheEntry(entry.first, entry.second);
+      const std::vector<std::pair<cc::TransferCacheEntryType, uint32_t>>&
+          entries) final {
+    gl_->UnlockTransferCacheEntries(entries);
   }
 
   GLES2Implementation* gl_;
@@ -6195,10 +6193,10 @@ bool GLES2Implementation::ThreadsafeLockTransferCacheEntry(
   return share_group()->transfer_cache()->LockTransferCacheEntry(type, id);
 }
 
-void GLES2Implementation::UnlockTransferCacheEntry(
-    cc::TransferCacheEntryType type,
-    uint32_t id) {
-  share_group()->transfer_cache()->UnlockTransferCacheEntry(helper_, type, id);
+void GLES2Implementation::UnlockTransferCacheEntries(
+    const std::vector<std::pair<cc::TransferCacheEntryType, uint32_t>>&
+        entries) {
+  share_group()->transfer_cache()->UnlockTransferCacheEntries(helper_, entries);
 }
 
 void GLES2Implementation::DeleteTransferCacheEntry(

gpu/command_buffer/client/gles2_implementation.h

@@ -221,8 +221,9 @@ class GLES2_IMPL_EXPORT GLES2Implementation
       const cc::ClientTransferCacheEntry& entry) override;
   bool ThreadsafeLockTransferCacheEntry(cc::TransferCacheEntryType type,
                                         uint32_t id) override;
-  void UnlockTransferCacheEntry(cc::TransferCacheEntryType type,
-                                uint32_t id) override;
+  void UnlockTransferCacheEntries(
+      const std::vector<std::pair<cc::TransferCacheEntryType, uint32_t>>&
+          entries) override;
   void DeleteTransferCacheEntry(cc::TransferCacheEntryType type,
                                 uint32_t id) override;