GPU Program Caching

gpu/command_buffer/service/program_cache_unittest.cc

Index: gpu/command_buffer/service/program_cache_unittest.cc
diff --git a/gpu/command_buffer/service/program_cache_unittest.cc b/gpu/command_buffer/service/program_cache_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1968cc207d089a89fa8021b18c9d8b4955cbc855
--- /dev/null
+++ b/gpu/command_buffer/service/program_cache_unittest.cc
@@ -0,0 +1,198 @@
+// 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/service/program_cache.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+typedef std::map<std::string, GLint> LocationMap;

[greggman, 2012/06/26 23:00:27]

sorry to harp on this but just to make it clear. It seems like this should go in
ProgramCache as in

class ProgramCache {
 public:
  typedef std::map<std::string, GLint> LocationMap;
..
}

It's part of it's interface so it needs to be defined in one place so that all
the overriding functions get the same signature everywhere.

[dmurph, 2012/07/04 00:01:29]

Done.

+}  // anonymous namespace
+
+namespace gpu {
+namespace gles2 {
+
+class NoBackendProgramCache : public ProgramCache {
+ public:
+  bool LoadLinkedProgram(const GLuint program,
+                         ShaderManager::ShaderInfo* shader_a,
+                         ShaderManager::ShaderInfo* shader_b,
+                         const LocationMap* bind_attrib_location_map) const {
+    return true;
+  }
+  void SaveLinkedProgram(const GLuint program,
+                         const ShaderManager::ShaderInfo* shader_a,
+                         const ShaderManager::ShaderInfo* shader_b,
+                         const LocationMap* bind_attrib_location_map) { }
+
+  void ClearBackend() {}
+
+  void SaySuccessfullyCached(const std::string& shader1,
+                               const std::string& shader2,
+                               std::map<std::string, GLint>* attrib_map) {
+    char a_sha[kHashLength];
+    char b_sha[kHashLength];
+    ComputeShaderHash(shader1, a_sha);
+    ComputeShaderHash(shader2, b_sha);
+
+    char sha[kHashLength];
+    ComputeProgramHash(a_sha,
+                       b_sha,
+                       attrib_map,
+                       sha);
+    const std::string shaString(sha, kHashLength);
+
+    LinkedProgramCacheSuccess(shaString,
+                        std::string(a_sha, kHashLength),

[greggman, 2012/06/26 23:00:27]

style: indenting

[dmurph, 2012/07/04 00:01:29]

Done.

+                        std::string(b_sha, kHashLength));
+  }
+
+  void ComputeShaderHash(const std::string& shader,
+                         char* result) const {
+    ProgramCache::ComputeShaderHash(shader, result);
+  }
+
+  void ComputeProgramHash(const char* hashed_shader_0,
+                          const char* hashed_shader_1,
+                          const LocationMap* bind_attrib_location_map,
+                          char* result) const {
+    ProgramCache::ComputeProgramHash(hashed_shader_0,
+                                     hashed_shader_1,
+                                     bind_attrib_location_map,
+                                     result);
+  }
+
+  void evict(const std::string& program_hash,

[greggman, 2012/06/26 23:00:27]

style: functions are UpperCase

+             const std::string& shader_0_hash,
+             const std::string& shader_1_hash) {
+    ProgramCache::Evict(program_hash, shader_0_hash, shader_1_hash);
+  }
+
+  const uint32 hash_length() const {

[greggman, 2012/06/26 23:00:27]

Is there a reason have this vs just using ProgramCache::kHashLength everywhere? 

[dmurph, 2012/07/04 00:01:29]

it was because I has kHashLength as a protected field, but I'll just make it
public

+    return ProgramCache::kHashLength;
+  }
+};
+
+class ProgramCacheTest : public testing::Test {
+ public:
+  ProgramCacheTest() :
+    cache_(new NoBackendProgramCache()) { }
+
+ protected:
+  scoped_ptr<NoBackendProgramCache> cache_;
+};
+
+TEST_F(ProgramCacheTest, ProgramCacheCompilationStatus) {
+  const std::string shader1 = "abcd1234";
+  {

[greggman, 2012/06/26 23:00:27]

what's this extra code block for?

[dmurph, 2012/07/04 00:01:29]

So the strings given will be destructed, to make sure the cache is doing a copy
or hash

+    std::string shader = shader1;
+    ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+              cache_->GetShaderCompilationStatus(shader));
+    cache_->ShaderCompilationSucceeded(shader);
+    shader.clear();

[greggman, 2012/06/26 23:00:27]

why are you clearing this string?

[dmurph, 2012/07/04 00:01:29]

same.  I can get rid of these checks if they seem overdone

+  }
+  // make sure it was copied
+  ASSERT_EQ(ProgramCache::COMPILATION_SUCCEEDED,
+            cache_->GetShaderCompilationStatus(shader1));
+}
+
+TEST_F(ProgramCacheTest, ProgramCacheProgramLinkStatus) {
+  const std::string shader1 = "abcd1234";
+  const std::string shader2 = "abcda sda b1~#4 bbbbb1234";
+  {

[greggman, 2012/06/26 23:00:27]

same as above. What's the extra code block for and why clear strings?

+    std::string shaderA = shader1;

[greggman, 2012/06/26 23:00:27]

style: variables are under_score

+    std::string shaderB = shader2;
+    ASSERT_EQ(ProgramCache::LINK_UNKNOWN,
+              cache_->GetLinkedProgramStatus(shaderA, shaderB, NULL));
+    cache_->SaySuccessfullyCached(shaderA, shaderB, NULL);
+
+    shaderA.clear();
+    shaderB.clear();
+  }
+  // make sure it was copied
+  ASSERT_EQ(ProgramCache::LINK_SUCCEEDED,
+            cache_->GetLinkedProgramStatus(shader1, shader2, NULL));
+}
+
+TEST_F(ProgramCacheTest, ProgramCacheEviction) {
+  const std::string shader1 = "abcd1234";
+  const std::string shader2 = "abcda sda b1~#4 bbbbb1234";
+  cache_->ShaderCompilationSucceeded(shader1);
+  cache_->ShaderCompilationSucceeded(shader2);
+  cache_->SaySuccessfullyCached(shader1, shader2, NULL);
+  const uint32 kHashLength = cache_->hash_length();
+  char a_sha[kHashLength];

[greggman, 2012/06/26 23:00:27]

dynamically sized c arrays are not standard c. This problem goes away if you
just use ProgramCache::kHashLength

[dmurph, 2012/07/04 00:01:29]

Done.

+  char b_sha[kHashLength];
+  cache_->ComputeShaderHash(shader1, a_sha);
+  cache_->ComputeShaderHash(shader2, b_sha);
+
+  char sha[kHashLength];
+  cache_->ComputeProgramHash(a_sha,
+                                     b_sha,

[greggman, 2012/06/26 23:00:27]

style: indenting

[dmurph, 2012/07/04 00:01:29]

Done.

+                                     NULL,
+                                     sha);
+  cache_->evict(std::string(sha, kHashLength),
+                        std::string(a_sha, kHashLength),

[greggman, 2012/06/26 23:00:27]

style: indenting

[dmurph, 2012/07/04 00:01:29]

Done.

+                        std::string(b_sha, kHashLength));
+  ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+            cache_->GetShaderCompilationStatus(shader1));
+  ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+            cache_->GetShaderCompilationStatus(shader2));
+  ASSERT_EQ(ProgramCache::LINK_UNKNOWN,
+            cache_->GetLinkedProgramStatus(shader1, shader2, NULL));
+}
+
+TEST_F(ProgramCacheTest, ProgramCacheEvictionWithReusedShader) {
+  const std::string shader1 = "abcd1234";
+  const std::string shader2 = "abcda sda b1~#4 bbbbb1234";
+  const std::string shader3 = "asbjbbjj239a";
+  cache_->ShaderCompilationSucceeded(shader1);
+  cache_->ShaderCompilationSucceeded(shader2);
+  cache_->SaySuccessfullyCached(shader1, shader2, NULL);
+  cache_->ShaderCompilationSucceeded(shader3);
+  cache_->SaySuccessfullyCached(shader1, shader3, NULL);
+
+  const uint32 kHashLength = cache_->hash_length();
+  char a_sha[kHashLength];
+  char b_sha[kHashLength];
+  char c_sha[kHashLength];
+  cache_->ComputeShaderHash(shader1, a_sha);
+  cache_->ComputeShaderHash(shader2, b_sha);
+  cache_->ComputeShaderHash(shader3, c_sha);
+
+  char sha[kHashLength];
+  cache_->ComputeProgramHash(a_sha,
+                             b_sha,
+                             NULL,
+                             sha);
+  cache_->evict(std::string(sha, kHashLength),
+                std::string(a_sha, kHashLength),
+                std::string(b_sha, kHashLength));
+  ASSERT_EQ(ProgramCache::COMPILATION_SUCCEEDED,
+            cache_->GetShaderCompilationStatus(shader1));
+  ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+            cache_->GetShaderCompilationStatus(shader2));
+  ASSERT_EQ(ProgramCache::COMPILATION_SUCCEEDED,
+            cache_->GetShaderCompilationStatus(shader3));
+  ASSERT_EQ(ProgramCache::LINK_UNKNOWN,
+            cache_->GetLinkedProgramStatus(shader1, shader2, NULL));
+
+  cache_->ComputeProgramHash(a_sha,
+                             c_sha,
+                             NULL,
+                             sha);
+  cache_->evict(std::string(sha, kHashLength),
+                std::string(a_sha, kHashLength),
+                std::string(c_sha, kHashLength));
+  ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+            cache_->GetShaderCompilationStatus(shader1));
+  ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+            cache_->GetShaderCompilationStatus(shader2));
+  ASSERT_EQ(ProgramCache::COMPILATION_UNKNOWN,
+            cache_->GetShaderCompilationStatus(shader3));
+}
+
+}  // namespace gles2
+}  // namespace gpu