More compact representation of the GL Query cache

gpu/command_buffer/client/query_tracker.cc

Index: gpu/command_buffer/client/query_tracker.cc
diff --git a/gpu/command_buffer/client/query_tracker.cc b/gpu/command_buffer/client/query_tracker.cc
index e215c16061bec6e160c1409e38a8d080068f4782..928a2a0bafb3d9143509eafae5f43905256dc298 100644
--- a/gpu/command_buffer/client/query_tracker.cc
+++ b/gpu/command_buffer/client/query_tracker.cc
@@ -18,6 +18,17 @@
 namespace gpu {
 namespace gles2 {
 
+QuerySyncManager::Bucket::Bucket(QuerySync* sync_mem,
+                                 int32 shm_id,
+                                 unsigned int shm_offset)
+    : syncs(sync_mem),
+      used_query_count(0),
+      shm_id(shm_id),
+      base_shm_offset(shm_offset) {
+}
+
+QuerySyncManager::Bucket::~Bucket() = default;

[reveman, 2015/05/11 17:13:07]

is "= default;" allowed/recommended now?

[Daniel Bratell, 2015/05/12 16:16:40]

It is allowed. The differences between

Foo::~Foo() {}
and
Foo::~Foo() = default

are minuscule but I believe that = default will in some cases generate better
code (since the compiler and type traits can determine that the constructor
isn't hand written) so I'm trying to get into the habit of using it.

I don't mind changing to {} though so leaving it for you to decide which one you
want here.

+
 QuerySyncManager::QuerySyncManager(MappedMemoryManager* manager)
     : mapped_memory_(manager) {
   DCHECK(manager);
@@ -33,7 +44,14 @@ QuerySyncManager::~QuerySyncManager() {
 
 bool QuerySyncManager::Alloc(QuerySyncManager::QueryInfo* info) {
   DCHECK(info);
-  if (free_queries_.empty()) {
+  Bucket* bucket = nullptr;
+  for (Bucket* bucket_candidate : buckets_) {
+    if (bucket_candidate->free_queries.size() != 0) {

[reveman, 2015/05/11 17:13:07]

s/free_queries.size() != 0/!free_queries.empty()/

[Daniel Bratell, 2015/05/12 16:16:40]

Done.

+      bucket = bucket_candidate;
+      break;
+    }
+  }
+  if (!bucket) {
     int32 shm_id;
     unsigned int shm_offset;
     void* mem = mapped_memory_->Alloc(
@@ -42,36 +60,32 @@ bool QuerySyncManager::Alloc(QuerySyncManager::QueryInfo* info) {
       return false;
     }
     QuerySync* syncs = static_cast<QuerySync*>(mem);
-    Bucket* bucket = new Bucket(syncs);
+    bucket = new Bucket(syncs, shm_id, shm_offset);
     buckets_.push_back(bucket);
-    for (size_t ii = 0; ii < kSyncsPerBucket; ++ii) {
-      free_queries_.push_back(QueryInfo(bucket, shm_id, shm_offset, syncs));
-      ++syncs;
-      shm_offset += sizeof(*syncs);
-    }
+    bucket->free_queries.reserve(kSyncsPerBucket);

[reveman, 2015/05/11 17:13:07]

Please use kSyncsPerBucket when creating the vector in the ctor instead.

[Daniel Bratell, 2015/05/12 16:16:40]

Done.

+    for (size_t ii = 0; ii < kSyncsPerBucket; ++ii)

[reveman, 2015/05/11 17:13:07]

maybe move this loop to the ctor too

[Daniel Bratell, 2015/05/12 16:16:40]

Done.

+      bucket->free_queries.push_back(ii);
   }
-  *info = free_queries_.front();
-  ++(info->bucket->used_query_count);
+
+  unsigned short index_in_bucket = bucket->free_queries.back();
+  uint32 shm_offset =
+      bucket->base_shm_offset + index_in_bucket * sizeof(QuerySync);
+  QuerySync* sync = bucket->syncs + index_in_bucket;
+  *info = QueryInfo(bucket, bucket->shm_id, shm_offset, sync);
+  bucket->used_query_count++;
   info->sync->Reset();
-  free_queries_.pop_front();
+  bucket->free_queries.pop_back();
   return true;
 }
 
 void QuerySyncManager::Free(const QuerySyncManager::QueryInfo& info) {
   DCHECK_GT(info.bucket->used_query_count, 0u);
   --(info.bucket->used_query_count);
-  free_queries_.push_back(info);
+  unsigned short index_in_bucket = info.sync - info.bucket->syncs;
+  info.bucket->free_queries.push_back(index_in_bucket);
 }
 
 void QuerySyncManager::Shrink() {
-  std::deque<QueryInfo> new_queue;
-  while (!free_queries_.empty()) {
-    if (free_queries_.front().bucket->used_query_count)
-      new_queue.push_back(free_queries_.front());
-    free_queries_.pop_front();
-  }
-  free_queries_.swap(new_queue);
-
   std::deque<Bucket*> new_buckets;
   while (!buckets_.empty()) {
     Bucket* bucket = buckets_.front();