More compact representation of the GL Query cache

gpu/command_buffer/client/query_tracker.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/client/query_tracker.h"
 
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
 #include <GLES2/gl2extchromium.h>
 
 #include "base/atomicops.h"
 #include "base/numerics/safe_conversions.h"
 #include "gpu/command_buffer/client/gles2_cmd_helper.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/client/mapped_memory.h"
 #include "gpu/command_buffer/common/time.h"
 
 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);
 }
 
 QuerySyncManager::~QuerySyncManager() {
   while (!buckets_.empty()) {
     mapped_memory_->Free(buckets_.front()->syncs);
     delete buckets_.front();
     buckets_.pop_front();
   }
 }
 
 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(
         kSyncsPerBucket * sizeof(QuerySync), &shm_id, &shm_offset);
     if (!mem) {
       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();
     if (bucket->used_query_count) {
       new_buckets.push_back(bucket);
     } else {
       mapped_memory_->Free(bucket->syncs);
       delete bucket;
     }
     buckets_.pop_front();
@@ skipping 167 matching lines @@
     }
 
     query_sync_manager_.Free(query->info_);
     it = removed_queries_.erase(it);
     delete query;
   }
 }
 
 }  // namespace gles2
 }  // namespace gpu