GPU Program Caching

gpu/command_buffer/service/shader_manager.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_SERVICE_SHADER_MANAGER_H_
 #define GPU_COMMAND_BUFFER_SERVICE_SHADER_MANAGER_H_
 
 #include <string>
 #include "base/basictypes.h"
 #include "base/hash_tables.h"
@@ skipping 16 matching lines @@
   // This is used to keep the source code for a shader. This is because in order
   // to emluate GLES2 the shaders will have to be re-written before passed to
   // the underlying OpenGL. But, when the user calls glGetShaderSource they
   // should get the source they passed in, not the re-written source.
   class GPU_EXPORT ShaderInfo : public base::RefCounted<ShaderInfo> {
    public:
     typedef scoped_refptr<ShaderInfo> Ref;
     typedef ShaderTranslator::VariableInfo VariableInfo;
 
     void UpdateSource(const char* source) {
+      // prevent the compiled source from getting deleted when source_ is reset

[greggman, 2012/07/16 19:41:24]

I'm confused by this implementation.

It seems like all you want is

1) When glCompileShader is called and shader compilation is put off (marked as
pending) copy source to compilation_source
2) When doing a pending compile, use compilation_source

the end. Not sure why all this other complication is needed which swapping
strings, calling release, extra checks in compilation_source or some of the
other code here. 

Am I missing something?

[dmurph, 2012/07/16 20:00:37]

I'm doing it slightly differently, and I think it ends up being much simpler
along with less memory usage.

1) Watch for the event where we update the source after we have already
compiled, and store source_ the compiled_source_ pointer so it's not destructed
when we update source_.

2) Reset compiled_source_ when we compile.

When we want to grab our compiled source, if compiled_source_ is populated then
we know to use that (source changed after compile), otherwise we use source_
(source not changed after compile)

+      if(!compilation_source_.get() && source_.get()) {

[greggman, 2012/07/16 21:57:26]

Okay then maybe a bunch of comments need to change? See other comments.

[dmurph, 2012/07/17 18:17:13]

Updated comment, simplified this statement

+        compilation_source_.reset(source_.release());
+      }
       source_.reset(source ? new std::string(source) : NULL);
       translated_source_.reset(NULL);
     }
 
     void UpdateTranslatedSource(const char* translated_source) {
       translated_source_.reset(
           translated_source ? new std::string(translated_source) : NULL);
     }
 
     GLuint service_id() const {
       return service_id_;
     }
 
     GLenum shader_type() const {
       return shader_type_;
     }
 
     const std::string* source() const {
       return source_.get();
     }
 
     const std::string* translated_source() const {
       return translated_source_.get();
     }
 
     void SetStatus(
         bool valid, const char* log,
         ShaderTranslatorInterface* translator);
 
+    bool pending_compilation() const {
+      return pending_cache_miss_compilation_;
+    }
+
+    // The source that was compiled for this shader

[greggman, 2012/07/16 21:57:26]

How about

// The source that was used when the user called CompileShader. 
// This is only used for a deferred compile.

And how about renaming it something like 
source_for_deferred_compile?

[dmurph, 2012/07/17 18:17:13]

How about deferred_compilation_source?

+    const std::string* compilation_source() const {
+      return compilation_source_.get() != NULL ?
+          compilation_source_.get() :
+          source_.get();
+    }
+
+    // Stores the current source as the compiled source

[greggman, 2012/07/16 21:57:26]

This comment does not match what this function does.

Also, the function name doesn't seem to describe what this function does. It
says it's Setting the source as it was compiled. But (a) it's not storing any
source and (b) there is no store on compile. There is store on deferred (or
rather store when compile is requested) but there is no store on true compile
which makes this comment confusing, at least to me. :-p

[dmurph, 2012/07/17 18:17:13]

Did some refactoring to try and help clear all of this up.  Let me know what you
think.

+    void SetSourceAsCompiled(bool pending_cache_miss_compilation) {

[greggman, 2012/07/16 21:57:26]

Also, there needs to be unit tests for these new functions in
shader_manager_unittest.cc

[dmurph, 2012/07/17 18:17:13]

Done.

+      pending_cache_miss_compilation_ = pending_cache_miss_compilation;
+      compilation_source_.reset();
+    }
+
     const VariableInfo* GetAttribInfo(const std::string& name) const;
     const VariableInfo* GetUniformInfo(const std::string& name) const;
 
     // If the original_name is not found, return NULL.
     const std::string* GetAttribMappedName(
         const std::string& original_name) const;
 
     const std::string* log_info() const {
       return log_info_.get();
     }
 
     bool IsValid() const {
       return valid_;
     }
 
     bool IsDeleted() const {
       return service_id_ == 0;
     }
 
     bool InUse() const {
       DCHECK_GE(use_count_, 0);
       return use_count_ != 0;
     }
 
+    // Used by program cache.
+    const ShaderTranslator::VariableMap& attrib_map() const {
+      return attrib_map_;
+    }
+
+    // Used by program cache.
+    const ShaderTranslator::VariableMap& uniform_map() const {
+      return uniform_map_;
+    }
+
+    // Used by program cache.
+    void set_attrib_map(const ShaderTranslator::VariableMap& attrib_map) {
+      // copied because cache might be cleared
+      attrib_map_ = ShaderTranslator::VariableMap(attrib_map);
+    }
+
+    // Used by program cache.
+    void set_uniform_map(const ShaderTranslator::VariableMap& uniform_map) {
+      // copied because cache might be cleared
+      uniform_map_ = ShaderTranslator::VariableMap(uniform_map);
+    }
+
    private:
     typedef ShaderTranslator::VariableMap VariableMap;
 
     friend class base::RefCounted<ShaderInfo>;
     friend class ShaderManager;
 
     ShaderInfo(GLuint service_id, GLenum shader_type);
     ~ShaderInfo();
 
     void IncUseCount();
@@ skipping 15 matching lines @@
 
     // The translated shader source.
     scoped_ptr<std::string> translated_source_;
 
     // The shader translation log.
     scoped_ptr<std::string> log_info_;
 
     // The type info when the shader was last compiled.
     VariableMap attrib_map_;
     VariableMap uniform_map_;
+
+    // If compilation was delayed for possible cache hit
+    bool pending_cache_miss_compilation_;
+
+    // Holds on to the source that was supposed to be compiled

[greggman, 2012/07/16 21:57:26]

This comment makes sense but I might be better to match it if you accept the
other suggested changes to something like
// Holds on to the source for a deferred compile.

[dmurph, 2012/07/17 18:17:13]

Done.

+    scoped_ptr<std::string> compilation_source_;
   };
 
   ShaderManager();
   ~ShaderManager();
 
   // Must call before destruction.
   void Destroy(bool have_context);
 
   // Creates a shader info for the given shader ID.
   ShaderInfo* CreateShaderInfo(
@@ skipping 28 matching lines @@
   void RemoveShaderInfoIfUnused(ShaderInfo* info);
 
   DISALLOW_COPY_AND_ASSIGN(ShaderManager);
 };
 
 }  // namespace gles2
 }  // namespace gpu
 
 #endif  // GPU_COMMAND_BUFFER_SERVICE_SHADER_MANAGER_H_