[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.arceri at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>