Fix slow TextureManager::StopTracking()

Iterating an ObserverList which uses WeakPtr items seems very slow.

Changing to std::vector<> gives about a 15X speedup.
Further skipping the observer call backs for TextureRefs with no current observers should cut this close to 0.

BUG=365536

Review URL: https://codereview.chromium.org/246353002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265469 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 53 insertions, 13 deletions

diff --git a/gpu/command_buffer/service/async_pixel_transfer_manager.cc b/gpu/command_buffer/service/async_pixel_transfer_manager.cc
index 3084dd6..efc893a 100644
--- a/gpu/command_buffer/service/async_pixel_transfer_manager.cc
+++ b/gpu/command_buffer/service/async_pixel_transfer_manager.cc
@@ -17,6 +17,12 @@ AsyncPixelTransferManager::AsyncPixelTransferManager() {}
 AsyncPixelTransferManager::~AsyncPixelTransferManager() {
   if (manager_)
     manager_->RemoveObserver(this);
+
+  for (TextureToDelegateMap::iterator ref = delegate_map_.begin();
+       ref != delegate_map_.end();
+       ref++) {
+    ref->first->RemoveObserver();
+  }
 }
 
 void AsyncPixelTransferManager::Initialize(gles2::TextureManager* manager) {
@@ -32,6 +38,7 @@ AsyncPixelTransferManager::CreatePixelTransferDelegate(
   AsyncPixelTransferDelegate* delegate =
       CreatePixelTransferDelegateImpl(ref, define_params);
   delegate_map_[ref] = make_linked_ptr(delegate);
+  ref->AddObserver();
   return delegate;
 }
 
@@ -49,8 +56,10 @@ AsyncPixelTransferManager::GetPixelTransferDelegate(
 void AsyncPixelTransferManager::ClearPixelTransferDelegateForTest(
     gles2::TextureRef* ref) {
   TextureToDelegateMap::iterator it = delegate_map_.find(ref);
-  if (it != delegate_map_.end())
+  if (it != delegate_map_.end()) {
     delegate_map_.erase(it);
+    ref->RemoveObserver();
+  }
 }
 
 bool AsyncPixelTransferManager::AsyncTransferIsInProgress(
@@ -69,8 +78,10 @@ void AsyncPixelTransferManager::OnTextureManagerDestroying(
 void AsyncPixelTransferManager::OnTextureRefDestroying(
     gles2::TextureRef* texture) {
   TextureToDelegateMap::iterator it = delegate_map_.find(texture);
-  if (it != delegate_map_.end())
+  if (it != delegate_map_.end()) {
     delegate_map_.erase(it);
+    texture->RemoveObserver();
+  }
 }
 
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index 8691dd1..9f2518a 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -8389,6 +8389,7 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
   // AsyncTexImage2D
   {
     // Create transfer state since it doesn't exist.
+    EXPECT_EQ(texture_ref->num_observers(), 0);
     EXPECT_CALL(*manager, CreatePixelTransferDelegateImpl(texture_ref, _))
         .WillOnce(Return(
             delegate = new StrictMock<gpu::MockAsyncPixelTransferDelegate>))
@@ -8408,6 +8409,7 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
     EXPECT_TRUE(texture->SafeToRenderFrom());
     GLsizei width, height;
     EXPECT_FALSE(texture->GetLevelSize(GL_TEXTURE_2D, 0, &width, &height));
+    EXPECT_EQ(texture_ref->num_observers(), 1);
   }
   {
     // Async redefinitions are not allowed!
@@ -8441,8 +8443,10 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
   }
 
   // AsyncTexSubImage2D
+  EXPECT_CALL(*delegate, Destroy()).RetiresOnSaturation();
   decoder_->GetAsyncPixelTransferManager()
       ->ClearPixelTransferDelegateForTest(texture_ref);
+  EXPECT_EQ(texture_ref->num_observers(), 0);
   texture->SetImmutable(false);
   {
     // Create transfer state since it doesn't exist.
@@ -8498,11 +8502,13 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
 
   // Delete delegate on DeleteTexture.
   {
+    EXPECT_EQ(texture_ref->num_observers(), 1);
     EXPECT_CALL(*delegate, Destroy()).RetiresOnSaturation();
     DoDeleteTexture(client_texture_id_, kServiceTextureId);
     EXPECT_FALSE(
         decoder_->GetAsyncPixelTransferManager()->GetPixelTransferDelegate(
             texture_ref));
+    EXPECT_EQ(texture_ref->num_observers(), 0);
     texture = NULL;
     texture_ref = NULL;
     delegate = NULL;
@@ -8581,6 +8587,11 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransfers) {
     EXPECT_EQ(error::kNoError, ExecuteCmd(wait_all_cmd));
     EXPECT_EQ(GL_NO_ERROR, GetGLError());
   }
+
+  // Remove PixelTransferManager before the decoder destroys.
+  EXPECT_CALL(*delegate, Destroy()).RetiresOnSaturation();
+  decoder_->ResetAsyncPixelTransferManagerForTest();
+  manager = NULL;
 }
 
 TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransferManager) {
@@ -8631,11 +8642,14 @@ TEST_F(GLES2DecoderManualInitTest, AsyncPixelTransferManager) {
 
   // Delete delegate on manager teardown.
   {
+    EXPECT_EQ(texture_ref->num_observers(), 1);
     EXPECT_CALL(*delegate, Destroy()).RetiresOnSaturation();
     decoder_->ResetAsyncPixelTransferManagerForTest();
+    manager = NULL;
 
     // Texture ref still valid.
     EXPECT_EQ(texture_ref, GetTexture(client_texture_id_));
+    EXPECT_EQ(texture_ref->num_observers(), 0);
   }
 }
 
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 8f7703b..c3a824c 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -66,9 +66,8 @@ TextureManager::DestructionObserver::DestructionObserver() {}
 TextureManager::DestructionObserver::~DestructionObserver() {}
 
 TextureManager::~TextureManager() {
-  FOR_EACH_OBSERVER(DestructionObserver,
-                    destruction_observers_,
-                    OnTextureManagerDestroying(this));
+  for (unsigned int i = 0; i < destruction_observers_.size(); i++)
+    destruction_observers_[i]->OnTextureManagerDestroying(this);
 
   DCHECK(textures_.empty());
 
@@ -873,7 +872,8 @@ TextureRef::TextureRef(TextureManager* manager,
                        Texture* texture)
     : manager_(manager),
       texture_(texture),
-      client_id_(client_id) {
+      client_id_(client_id),
+      num_observers_(0) {
   DCHECK(manager_);
   DCHECK(texture_);
   texture_->AddTextureRef(this);
@@ -1222,9 +1222,12 @@ void TextureManager::StartTracking(TextureRef* ref) {
 }
 
 void TextureManager::StopTracking(TextureRef* ref) {
-  FOR_EACH_OBSERVER(DestructionObserver,
-                    destruction_observers_,
-                    OnTextureRefDestroying(ref));
+  if (ref->num_observers()) {
+    for (unsigned int i = 0; i < destruction_observers_.size(); i++) {
+      destruction_observers_[i]->OnTextureRefDestroying(ref);
+    }
+    DCHECK_EQ(ref->num_observers(), 0);
+  }
 
   Texture* texture = ref->texture();
 
diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h
index 8513264..6b3a32e 100644
--- a/gpu/command_buffer/service/texture_manager.h
+++ b/gpu/command_buffer/service/texture_manager.h
@@ -13,7 +13,6 @@
 #include "base/containers/hash_tables.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
-#include "base/observer_list.h"
 #include "gpu/command_buffer/service/async_pixel_transfer_delegate.h"
 #include "gpu/command_buffer/service/gl_utils.h"
 #include "gpu/command_buffer/service/memory_tracking.h"
@@ -404,10 +403,15 @@ class GPU_EXPORT TextureRef : public base::RefCounted<TextureRef> {
   static scoped_refptr<TextureRef> Create(TextureManager* manager,
                                           GLuint client_id,
                                           GLuint service_id);
+
+  void AddObserver() { num_observers_++; }
+  void RemoveObserver() { num_observers_--; }
+
   const Texture* texture() const { return texture_; }
   Texture* texture() { return texture_; }
   GLuint client_id() const { return client_id_; }
   GLuint service_id() const { return texture_->service_id(); }
+  GLint num_observers() const { return num_observers_; }
 
  private:
   friend class base::RefCounted<TextureRef>;
@@ -422,6 +426,7 @@ class GPU_EXPORT TextureRef : public base::RefCounted<TextureRef> {
   TextureManager* manager_;
   Texture* texture_;
   GLuint client_id_;
+  GLint num_observers_;
 
   DISALLOW_COPY_AND_ASSIGN(TextureRef);
 };
@@ -682,11 +687,18 @@ class GPU_EXPORT TextureManager {
       std::string* signature) const;
 
   void AddObserver(DestructionObserver* observer) {
-    destruction_observers_.AddObserver(observer);
+    destruction_observers_.push_back(observer);
   }
 
   void RemoveObserver(DestructionObserver* observer) {
-    destruction_observers_.RemoveObserver(observer);
+    for (unsigned int i = 0; i < destruction_observers_.size(); i++) {
+      if (destruction_observers_[i] == observer) {
+        std::swap(destruction_observers_[i], destruction_observers_.back());
+        destruction_observers_.pop_back();
+        return;
+      }
+    }
+    NOTREACHED();
   }
 
   struct DoTextImage2DArguments {
@@ -796,7 +808,7 @@ class GPU_EXPORT TextureManager {
   // The default textures for each target (texture name = 0)
   scoped_refptr<TextureRef> default_textures_[kNumDefaultTextures];
 
-  ObserverList<DestructionObserver> destruction_observers_;
+  std::vector<DestructionObserver*> destruction_observers_;
 
   DISALLOW_COPY_AND_ASSIGN(TextureManager);
 };