More compact representation of the GL Query cache

gpu/command_buffer/client/query_tracker.h

 // 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.
 
 #ifndef GPU_COMMAND_BUFFER_CLIENT_QUERY_TRACKER_H_
 #define GPU_COMMAND_BUFFER_CLIENT_QUERY_TRACKER_H_
 
 #include <GLES2/gl2.h>
 
 #include <deque>
 #include <list>
+#include <vector>
 
 #include "base/atomicops.h"
 #include "base/containers/hash_tables.h"
 #include "gles2_impl_export.h"
 #include "gpu/command_buffer/common/gles2_cmd_format.h"
 
 namespace gpu {
 
 class CommandBufferHelper;
 class MappedMemoryManager;
 
 namespace gles2 {
 
 class GLES2Implementation;
 
 // Manages buckets of QuerySync instances in mapped memory.
 class GLES2_IMPL_EXPORT QuerySyncManager {
  public:
   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)
         : bucket(bucket),
           shm_id(id),
           shm_offset(offset),
           sync(sync_mem) {
     }
 
     QueryInfo()
@@ skipping 12 matching lines @@
   explicit QuerySyncManager(MappedMemoryManager* manager);
   ~QuerySyncManager();
 
   bool Alloc(QueryInfo* info);
   void Free(const QueryInfo& sync);
   void Shrink();
 
  private:
   MappedMemoryManager* mapped_memory_;
   std::deque<Bucket*> buckets_;
-  std::deque<QueryInfo> free_queries_;
 
   DISALLOW_COPY_AND_ASSIGN(QuerySyncManager);
 };
 
 // Tracks queries for client side of command buffer.
 class GLES2_IMPL_EXPORT QueryTracker {
  public:
   class GLES2_IMPL_EXPORT Query {
    public:
     enum State {
@@ skipping 86 matching lines @@
   QueryList removed_queries_;
   QuerySyncManager query_sync_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(QueryTracker);
 };
 
 }  // namespace gles2
 }  // namespace gpu
 
 #endif  // GPU_COMMAND_BUFFER_CLIENT_QUERY_TRACKER_H_