gpu: Fix removal of pending queries.

This moves all queries to a "removed queries" list at removal
time. The memory used by the queries is not freed until the
query has completed.

BUG=253488
TEST=gpu_unittests --gtest_filter=QueryTrackerTest.Remove
NOTRY=true

Review URL: https://chromiumcodereview.appspot.com/18340003

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

4 files changed, 79 insertions, 36 deletions

diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index fbc92d5..1bc3f0b3 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -3174,36 +3174,10 @@ void GLES2Implementation::DeleteQueriesEXTHelper(
         "glDeleteTextures", "id not created by this context.");
     return;
   }
-  // When you delete a query you can't mark its memory as unused until it's
-  // completed.
-  // Note: If you don't do this you won't mess up the service but you will mess
-  // up yourself.
-
-  // TODO(gman): Consider making this faster by putting pending quereies
-  // on some queue to be removed when they are finished.
-  bool query_pending = false;
-  for (GLsizei ii = 0; ii < n; ++ii) {
-    QueryTracker::Query* query = query_tracker_->GetQuery(queries[ii]);
-    if (query && query->Pending()) {
-      query_pending = true;
-      break;
-    }
-  }
 
-  if (query_pending) {
-    WaitForCmd();
-  }
+  for (GLsizei ii = 0; ii < n; ++ii)
+    query_tracker_->RemoveQuery(queries[ii]);
 
-  for (GLsizei ii = 0; ii < n; ++ii) {
-    QueryTracker::Query* query = query_tracker_->GetQuery(queries[ii]);
-    if (query && query->Pending()) {
-      if (!query->CheckResultsAvailable(helper_)) {
-        // Should only get here on context lost.
-        MustBeContextLost();
-      }
-    }
-    query_tracker_->RemoveQuery(queries[ii], helper_->IsContextLost());
-  }
   helper_->DeleteQueriesEXTImmediate(n, queries);
 }
 
diff --git a/gpu/command_buffer/client/query_tracker.cc b/gpu/command_buffer/client/query_tracker.cc
index 76ebf72..7cdc7fd 100644
--- a/gpu/command_buffer/client/query_tracker.cc
+++ b/gpu/command_buffer/client/query_tracker.cc
@@ -199,11 +199,19 @@ QueryTracker::QueryTracker(MappedMemoryManager* manager)
 }
 
 QueryTracker::~QueryTracker() {
-  queries_.clear();
+  while (!queries_.empty()) {
+    delete queries_.begin()->second;
+    queries_.erase(queries_.begin());
+  }
+  while (!removed_queries_.empty()) {
+    delete removed_queries_.front();
+    removed_queries_.pop_front();
+  }
 }
 
 QueryTracker::Query* QueryTracker::CreateQuery(GLuint id, GLenum target) {
   GPU_DCHECK_NE(0u, id);
+  FreeCompletedQueries();
   QuerySyncManager::QueryInfo info;
   if (!query_sync_manager_.Alloc(&info)) {
     return NULL;
@@ -221,21 +229,40 @@ QueryTracker::Query* QueryTracker::GetQuery(
   return it != queries_.end() ? it->second : NULL;
 }
 
-void QueryTracker::RemoveQuery(GLuint client_id, bool context_lost) {
-  (void)context_lost;  // stop unused warning
+void QueryTracker::RemoveQuery(GLuint client_id) {
   QueryMap::iterator it = queries_.find(client_id);
   if (it != queries_.end()) {
     Query* query = it->second;
-    GPU_DCHECK(context_lost || !query->Pending());
-    query_sync_manager_.Free(query->info_);
+    // When you delete a query you can't mark its memory as unused until it's
+    // completed.
+    // Note: If you don't do this you won't mess up the service but you will
+    // mess up yourself.
+    removed_queries_.push_back(query);
     queries_.erase(it);
-    delete query;
+    FreeCompletedQueries();
   }
 }
 
 void QueryTracker::Shrink() {
+  FreeCompletedQueries();
   query_sync_manager_.Shrink();
 }
 
+void QueryTracker::FreeCompletedQueries() {
+  QueryList::iterator it = removed_queries_.begin();
+  while (it != removed_queries_.end()) {
+    Query* query = *it;
+    if (query->Pending() &&
+        query->info_.sync->process_count != query->submit_count()) {
+      ++it;
+      continue;
+    }
+
+    query_sync_manager_.Free(query->info_);
+    it = removed_queries_.erase(it);
+    delete query;
+  }
+}
+
 }  // namespace gles2
 }  // namespace gpu
diff --git a/gpu/command_buffer/client/query_tracker.h b/gpu/command_buffer/client/query_tracker.h
index 144ae92..81861c9 100644
--- a/gpu/command_buffer/client/query_tracker.h
+++ b/gpu/command_buffer/client/query_tracker.h
@@ -8,6 +8,8 @@
 #include <GLES2/gl2.h>
 
 #include <deque>
+#include <list>
+
 #include "gles2_impl_export.h"
 #include "gpu/command_buffer/client/hash_tables.h"
 #include "gpu/command_buffer/common/gles2_cmd_format.h"
@@ -154,13 +156,16 @@ class GLES2_IMPL_EXPORT QueryTracker {
 
   Query* CreateQuery(GLuint id, GLenum target);
   Query* GetQuery(GLuint id);
-  void RemoveQuery(GLuint id, bool context_lost);
+  void RemoveQuery(GLuint id);
   void Shrink();
+  void FreeCompletedQueries();
 
  private:
   typedef gpu::hash_map<GLuint, Query*> QueryMap;
+  typedef std::list<Query*> QueryList;
 
   QueryMap queries_;
+  QueryList removed_queries_;
   QuerySyncManager query_sync_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(QueryTracker);
diff --git a/gpu/command_buffer/client/query_tracker_unittest.cc b/gpu/command_buffer/client/query_tracker_unittest.cc
index 82f807c..2dad335 100644
--- a/gpu/command_buffer/client/query_tracker_unittest.cc
+++ b/gpu/command_buffer/client/query_tracker_unittest.cc
@@ -96,6 +96,10 @@ class QueryTrackerTest : public testing::Test {
     return query->info_.sync;
   }
 
+  QuerySyncManager::Bucket* GetBucket(QueryTracker::Query* query) {
+    return query->info_.bucket;
+  }
+
   scoped_ptr<CommandBuffer> command_buffer_;
   scoped_ptr<GLES2CmdHelper> helper_;
   scoped_ptr<MappedMemoryManager> mapped_memory_;
@@ -115,7 +119,7 @@ TEST_F(QueryTrackerTest, Basic) {
   // Check we get nothing for a non-existent query.
   EXPECT_TRUE(query_tracker_->GetQuery(kId2) == NULL);
   // Check we can delete the query.
-  query_tracker_->RemoveQuery(kId1, false);
+  query_tracker_->RemoveQuery(kId1);
   // Check we get nothing for a non-existent query.
   EXPECT_TRUE(query_tracker_->GetQuery(kId1) == NULL);
 }
@@ -165,6 +169,39 @@ TEST_F(QueryTrackerTest, Query) {
   EXPECT_FALSE(query->Pending());
 }
 
+TEST_F(QueryTrackerTest, Remove) {
+  const GLuint kId1 = 123;
+  const int32 kToken = 46;
+  const uint32 kResult = 456;
+
+  // Create a Query.
+  QueryTracker::Query* query = query_tracker_->CreateQuery(
+      kId1, GL_ANY_SAMPLES_PASSED_EXT);
+  ASSERT_TRUE(query != NULL);
+
+  QuerySyncManager::Bucket* bucket = GetBucket(query);
+  EXPECT_EQ(1u, bucket->used_query_count);
+
+  query->MarkAsActive();
+  query->MarkAsPending(kToken);
+
+  query_tracker_->RemoveQuery(kId1);
+  // Check we get nothing for a non-existent query.
+  EXPECT_TRUE(query_tracker_->GetQuery(kId1) == NULL);
+
+  // Check that memory was not freed.
+  EXPECT_EQ(1u, bucket->used_query_count);
+
+  // Simulate GPU process marking it as available.
+  QuerySync* sync = GetSync(query);
+  sync->process_count = query->submit_count();
+  sync->result = kResult;
+
+  // Check FreeCompletedQueries.
+  query_tracker_->FreeCompletedQueries();
+  EXPECT_EQ(0u, bucket->used_query_count);
+}
+
 }  // namespace gles2
 }  // namespace gpu