GPU Program Caching

gpu/command_buffer/service/program_cache.h

Index: gpu/command_buffer/service/program_cache.h
diff --git a/gpu/command_buffer/service/program_cache.h b/gpu/command_buffer/service/program_cache.h
new file mode 100644
index 0000000000000000000000000000000000000000..fb584b58f50ec5a3c59fb7dd3459c5b424eb6a11
--- /dev/null
+++ b/gpu/command_buffer/service/program_cache.h
@@ -0,0 +1,130 @@
+// 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_SERVICE_program_cache_H_
+#define GPU_COMMAND_BUFFER_SERVICE_program_cache_H_
+#pragma once
+
+#include <map>
+
+#include "base/hash_tables.h"
+#include "base/memory/weak_ptr.h"

[greggman, 2012/06/25 18:53:03]

is this still needed?

[dmurph, 2012/06/26 02:32:56]

Done.

+#include "base/sha1.h"
+#include "gpu/command_buffer/common/gles2_cmd_format.h"
+#include "gpu/command_buffer/service/shader_manager.h"
+#include "net/disk_cache/hash.h"
+
+namespace {
+typedef std::map<std::string, GLint> BindAttribMap;

[greggman, 2012/06/25 18:53:03]

I don't think it makes sense to have an anonymous namespace in a header. I
probably didn't notice this was a header before.

Should be moved this into ProgramCache?

[dmurph, 2012/06/26 02:32:56]

It's just to shorten arguments for the map, I'll just expand them

+} // anonymous namespace
+
+namespace gpu {
+namespace gles2 {
+
+// Program cache base class for caching linked gpu programs
+class GPU_EXPORT ProgramCache {
+ public:
+  enum CompiledShaderStatus {
+    COMPILATION_UNKNOWN,
+    COMPILATION_SUCCEEDED
+  };
+
+  enum LinkedProgramStatus {
+    LINK_UNKNOWN,
+    LINK_SUCCEEDED
+  };
+
+  ProgramCache() {}
+  virtual ~ProgramCache();
+
+  CompiledShaderStatus GetShaderCompilationStatus(
+      const std::string& shader_src) const;
+  void ShaderCompilationSucceeded(const std::string& shader_src);
+
+  LinkedProgramStatus GetLinkedProgramStatus(
+      const std::string& untranslated_a,
+      const std::string& untranslated_b,
+      const BindAttribMap* bind_attrib_location_map) const;
+
+  virtual bool LoadLinkedProgram(
+      const GLuint program,
+      ShaderManager::ShaderInfo* shader_a,
+      ShaderManager::ShaderInfo* shader_b,
+      const BindAttribMap* bind_attrib_location_map) const = 0;
+  virtual void SaveLinkedProgram(
+      const GLuint program,
+      const ShaderManager::ShaderInfo* shader_a,
+      const ShaderManager::ShaderInfo* shader_b,
+      const BindAttribMap* bind_attrib_location_map) = 0;
+  // clears the cache
+  void Clear();
+
+ protected:
+  static const uint32 kHashLength = base::kSHA1Length;

[greggman, 2012/06/25 18:53:03]

Is there a reason to use uint32 everywhere? the style guide prefers 'size_t'
where appropriate and 'int' otherwise.

uint32 should only be used if it absolutely needs to be uint32 (4 bytes)

[dmurph, 2012/06/26 02:32:56]

Done.

+
+  // called by implementing class after a shader was successfully cached
+  void LinkedProgramCacheSuccess(const std::string& program_hash,
+                                 const std::string& shader_a_hash,
+                                 const std::string& shader_b_hash);
+
+  // result is not null terminated
+  void ComputeShaderHash(const std::string& shader,
+                         char* result) const;
+
+  // result is not null terminated.  hashed shaders are expected to be
+  // kHashLength in length
+  void ComputeProgramHash(const char* hashed_shader_0,
+                          const char* hashed_shader_1,
+                          const BindAttribMap* bind_attrib_location_map,
+                          char* result) const;
+
+  void Evict(const std::string& program_hash,
+             const std::string& shader_0_hash,
+             const std::string& shader_1_hash);
+
+ private:
+  struct FastHash {
+    const inline uint32 operator()(const std::string& key) const {
+      if (key.empty())
+        return 0;
+      return disk_cache::SuperFastHash(key.data(),
+                                       static_cast<int>(key.size()));
+    }
+  };
+  struct CompiledShaderInfo {

[greggman, 2012/06/25 18:53:03]

We don't usually allow structs to have methods that do anything. Should be a
class?

[dmurph, 2012/06/26 02:32:56]

I'll remove the method

+    CompiledShaderInfo() : status(COMPILATION_UNKNOWN), ref_count(0) { }
+    CompiledShaderInfo(CompiledShaderStatus status_)
+        : status(status_),
+          ref_count(0) { }
+
+    void usedInCachedProgram() {

[greggman, 2012/06/25 18:53:03]

If you're going to have a function to inc ref count should you have one to dec
ref count?

Better yet, DecerementRefCount() or UnuseInCachedProgram()  can do a
DCHECK_NE(0, ref_count); and ref_count_ can be private.

[dmurph, 2012/06/26 02:32:56]

Ends up not being needed, that check is included on eviction

+      ref_count++;
+    }
+
+    CompiledShaderStatus status;
+    uint32 ref_count;
+  };
+
+  typedef base::hash_map<std::string,
+                          CompiledShaderInfo,
+                          FastHash> CompileStatusMap;
+  typedef base::hash_map<std::string,
+                        LinkedProgramStatus,
+                        FastHash> LinkStatusMap;
+
+  // called to clear the backend cache
+  virtual void ClearBackend() = 0;

[greggman, 2012/06/25 18:53:03]

There's no such thing as a pure virtual private method is there?

[dmurph, 2012/06/26 02:32:56]

Yeah, it's so the derived class can provide the implementation, but they can't
call the method.
http://stackoverflow.com/questions/2170688/private-virtual-method-in-c

+
+  CompileStatusMap shader_status_;
+  LinkStatusMap link_status_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProgramCache);
+};
+
+
+
+} // namespace gles2
+} // namespace gpu
+
+#endif // GPU_COMMAND_BUFFER_SERVICE_program_cache_H_