More compact representation of the GL Query cache

gpu/command_buffer/client/query_tracker.h

Index: gpu/command_buffer/client/query_tracker.h
diff --git a/gpu/command_buffer/client/query_tracker.h b/gpu/command_buffer/client/query_tracker.h
index 72e29e71c421b007b0a9405c6fad615c551dc6e3..a894f81901338ee656c2358d1abba67a1a4bb856 100644
--- a/gpu/command_buffer/client/query_tracker.h
+++ b/gpu/command_buffer/client/query_tracker.h
@@ -9,6 +9,7 @@
 
 #include <deque>
 #include <list>
+#include <vector>
 
 #include "base/atomicops.h"
 #include "base/containers/hash_tables.h"
@@ -30,12 +31,13 @@ class GLES2_IMPL_EXPORT QuerySyncManager {
   static const size_t kSyncsPerBucket = 4096;
 
   struct Bucket {
-    explicit Bucket(QuerySync* sync_mem)
-        : syncs(sync_mem),
-          used_query_count(0) {
-    }
+    Bucket(QuerySync* sync_mem, int32 shm_id, unsigned int shm_offset);
+    ~Bucket();
     QuerySync* syncs;
     unsigned used_query_count;

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

is |used_query_count| still needed?

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

It can be replaced by |kSyncsPerBucket - bucket->free_queries.size()| but I
didn't want to make the patch more complicated so I left if. I wouldn't mind
doing that change if you think it's useful. For memory usage it would save 4
bytes per QueryBucket but probably add some 10-30 bytes of machine code.

[reveman, 2015/05/12 16:45:05]

I generally prefer unnecessary state when possible but I'll leave it up to you
to decide.

+    int32 shm_id;
+    unsigned int base_shm_offset;

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

nit: uint32 base_shm_offset to be consistent with SyncInfo

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

Done.

+    std::vector<unsigned short> free_queries;

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

can we just use vector<size_t> here instead? that's the type used for
kSyncsPerBucket...

[jbauman, 2015/05/11 19:12:53]

Could you add a COMPILE_ASSERT that kSyncsPerBucket <= USHRT_MAX (or similar
constant)?

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

This is the main source of memory usage in the cache so it's valuable to use the
smallest type possible. I will add a static assert as jbauman suggests so that
we don't get bitten by it if the cache is grown to more than 64k slots.

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

Done.

[reveman, 2015/05/12 16:45:05]

Acknowledged.

Btw, have you considered using std::bitset<kSyncsPerBucket> where bit k
represents the query at index k? That would very compact and I bet it would be
plenty fast to just search it for a free query.

   };
   struct QueryInfo {
     QueryInfo(Bucket* bucket, int32 id, uint32 offset, QuerySync* sync_mem)
@@ -68,7 +70,6 @@ class GLES2_IMPL_EXPORT QuerySyncManager {
  private:
   MappedMemoryManager* mapped_memory_;
   std::deque<Bucket*> buckets_;
-  std::deque<QueryInfo> free_queries_;
 
   DISALLOW_COPY_AND_ASSIGN(QuerySyncManager);
 };