Re: [Mesa-dev] [PATCH 07/40] glsl: make use of on disk shader cache

[Nicolai Hähnle]

On 10.02.2017 13:35, Timothy Arceri wrote:
On 10/02/17 22:41, Nicolai Hähnle wrote:
On 07.02.2017 04:42, Timothy Arceri wrote:
From: Timothy Arceri <timothy.arc...@collabora.com>
The hash key for glsl metadata is a hash of the hashes of each GLSL
source string.

This commit uses the put_key/get_key support in the cache put the SHA-1
hash of the source string for each successfully compiled shader into the
cache. This allows for early, optimistic returns from glCompileShader
(if the identical source string had been successfully compiled in the
past),
in the hope that the final, linked shader will be found in the cache.

This is based on the intial patch by Carl.

Okay, here I actually think you may need some fallback handling around
the shader source. I'm thinking of the following sequence:

glShaderSource
glCompileShader <-- deferred due to cache hit of shader

glShaderSource

glAttachShader
glLinkProgram <-- no cache hit for program

At that point, we have to actually compile the shader, but the true
shader source has been thrown away.

Or maybe I'm reading all of this code wrong?

Patch 22 should take care of this I think. Sorry for the patch ordering,
this patch should probably be at the end.

I don't think it does, at least not for st/mesa. Patch 22 ensures that 
the fallback source is stored, but I don't see it being actually used in 
the cache_fallback path.

Cheers,
Nicolai




Thanks,
Nicolai

---
 src/compiler/glsl/glsl_parser_extras.cpp | 16 ++++++++++++++++
 src/compiler/glsl/linker.cpp             |  5 +++++
 src/mesa/program/ir_to_mesa.cpp          |  8 ++++++++
 3 files changed, 29 insertions(+)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
b/src/compiler/glsl/glsl_parser_extras.cpp
index 6fe1dd9..e16d543 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -33,6 +33,8 @@
 #include "main/shaderobj.h"
 #include "util/u_atomic.h" /* for p_atomic_cmpxchg */
 #include "util/ralloc.h"
+#include "util/disk_cache.h"
+#include "util/mesa-sha1.h"
 #include "ast.h"
 #include "glsl_parser_extras.h"
 #include "glsl_parser.h"
@@ -1924,6 +1926,20 @@ _mesa_glsl_compile_shader(struct gl_context
*ctx, struct gl_shader *shader,
    state->error = glcpp_preprocess(state, &source, &state->info_log,
                              add_builtin_defines, state, ctx);

+   if (!force_recompile) {
+      char buf[41];
+      _mesa_sha1_compute(source, strlen(source), shader->sha1);
+      if (ctx->Cache && disk_cache_has_key(ctx->Cache, shader->sha1)) {
+         /* We've seen this shader before and know it compiles */
+         if (ctx->_Shader->Flags & GLSL_CACHE_INFO) {
+            fprintf(stderr, "deferring compile of shader: %s\n",
+                    _mesa_sha1_format(buf, shader->sha1));
+         }
+         shader->CompileStatus = true;
+         return;
+      }
+   }
+
    if (!state->error) {
      _mesa_glsl_lexer_ctor(state, source);
      _mesa_glsl_parse(state);
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 720c22b..b5745ba 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -73,6 +73,7 @@
 #include "program.h"
 #include "program/prog_instruction.h"
 #include "program/program.h"
+#include "util/mesa-sha1.h"
 #include "util/set.h"
 #include "util/string_to_uint_map.h"
 #include "linker.h"
@@ -81,6 +82,7 @@
 #include "ir_rvalue_visitor.h"
 #include "ir_uniform.h"
 #include "builtin_functions.h"
+#include "shader_cache.h"

 #include "main/shaderobj.h"
 #include "main/enums.h"
@@ -4651,6 +4653,9 @@ link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
       return;
    }

+   if (shader_cache_read_program_metadata(ctx, prog))
+      return;
+
    void *mem_ctx = ralloc_context(NULL); // temporary linker context

    prog->ARB_fragment_coord_conventions_enable = false;
diff --git a/src/mesa/program/ir_to_mesa.cpp
b/src/mesa/program/ir_to_mesa.cpp
index ce58fbb..67c9267 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -46,6 +46,7 @@
 #include "compiler/glsl_types.h"
 #include "compiler/glsl/linker.h"
 #include "compiler/glsl/program.h"
+#include "compiler/glsl/shader_cache.h"
 #include "program/prog_instruction.h"
 #include "program/prog_optimize.h"
 #include "program/prog_print.h"
@@ -3114,6 +3115,10 @@ _mesa_glsl_link_shader(struct gl_context *ctx,
struct gl_shader_program *prog)
       }
    }

+   /* Return early if we are loading the shader from on-disk cache */
+   if (prog->data->LinkStatus == linking_skipped)
+      return;
+
    if (ctx->_Shader->Flags & GLSL_DUMP) {
       if (!prog->data->LinkStatus) {
      fprintf(stderr, "GLSL shader program %d failed to link\n",
prog->Name);
@@ -3124,6 +3129,9 @@ _mesa_glsl_link_shader(struct gl_context *ctx,
struct gl_shader_program *prog)
          fprintf(stderr, "%s\n", prog->data->InfoLog);
       }
    }
+
+   if (prog->data->LinkStatus)
+      shader_cache_write_program_metadata(ctx, prog);
 }

 } /* extern "C" */


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev