[Mesa-dev] [PATCH V3 2/2] glsl: don't run the GLSL pre-processor when we are skipping compilation

[Dieter Nützel]

Tested-by (v3): Dieter Nützel <Dieter at nuetzel-hh.de>

I'm running 'all the time' with this.

Am 10.04.2017 03:48, schrieb Timothy Arceri:
> Improves Deus Ex start-up times with a warm cache from ~30 seconds to
> ~22 seconds.
> 
> Also fixes the leaking of state.
> 
> V2: fix indentation
> 
> v3: add the value of MESA_EXTENSION_OVERRIDE to the hash of the shader.
> 
> Tested-by (v2): Grazvydas Ignotas <notasas at gmail.com>
> ---
>  src/compiler/glsl/glsl_parser_extras.cpp | 19 ++++++++++---------
>  src/compiler/glsl/shader_cache.cpp       | 10 ++++++++++
>  2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index ca74b55..eb12eff 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1998,32 +1998,23 @@ opt_shader_and_create_symbol_table(struct
> gl_context *ctx,
>        }
>     }
> 
>     _mesa_glsl_initialize_derived_variables(ctx, shader);
>  }
> 
>  void
>  _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader 
> *shader,
>                            bool dump_ast, bool dump_hir, bool 
> force_recompile)
>  {
> -   struct _mesa_glsl_parse_state *state =
> -      new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
>     const char *source = force_recompile && shader->FallbackSource ?
>        shader->FallbackSource : shader->Source;
> 
> -   if (ctx->Const.GenerateTemporaryNames)
> -      (void) 
> p_atomic_cmpxchg(&ir_variable::temporaries_allocate_names,
> -                              false, true);
> -
> -   state->error = glcpp_preprocess(state, &source, &state->info_log,
> -                             add_builtin_defines, state, ctx);
> -
>     if (!force_recompile) {
>        if (ctx->Cache) {
>           char buf[41];
>           disk_cache_compute_key(ctx->Cache, source, strlen(source),
>                                  shader->sha1);
>           if (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) {
>                 _mesa_sha1_format(buf, shader->sha1);
>                 fprintf(stderr, "deferring compile of shader: %s\n", 
> buf);
> @@ -2043,20 +2034,30 @@ _mesa_glsl_compile_shader(struct gl_context
> *ctx, struct gl_shader *shader,
>        if (shader->CompileStatus == compile_success)
>           return;
> 
>        if (shader->CompileStatus == compiled_no_opts) {
>           opt_shader_and_create_symbol_table(ctx, shader);
>           shader->CompileStatus = compile_success;
>           return;
>        }
>     }
> 
> +   struct _mesa_glsl_parse_state *state =
> +      new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
> +
> +   if (ctx->Const.GenerateTemporaryNames)
> +      (void) 
> p_atomic_cmpxchg(&ir_variable::temporaries_allocate_names,
> +                              false, true);
> +
> +   state->error = glcpp_preprocess(state, &source, &state->info_log,
> +                                   add_builtin_defines, state, ctx);
> +
>     if (!state->error) {
>       _mesa_glsl_lexer_ctor(state, source);
>       _mesa_glsl_parse(state);
>       _mesa_glsl_lexer_dtor(state);
>       do_late_parsing_checks(state);
>     }
> 
>     if (dump_ast) {
>        foreach_list_typed(ast_node, ast, link, 
> &state->translation_unit) {
>           ast->print();
> diff --git a/src/compiler/glsl/shader_cache.cpp
> b/src/compiler/glsl/shader_cache.cpp
> index e51fecd..738e548 100644
> --- a/src/compiler/glsl/shader_cache.cpp
> +++ b/src/compiler/glsl/shader_cache.cpp
> @@ -1312,20 +1312,30 @@ shader_cache_read_program_metadata(struct
> gl_context *ctx,
>                            prog->SeparateShader ? "T" : "F");
> 
>     /* A shader might end up producing different output depending on 
> the glsl
>      * version supported by the compiler. For example a different path 
> might be
>      * taken by the preprocessor, so add the version to the hash input.
>      */
>     ralloc_asprintf_append(&buf, "api: %d glsl: %d fglsl: %d\n",
>                            ctx->API, ctx->Const.GLSLVersion,
>                            ctx->Const.ForceGLSLVersion);
> 
> +   /* We run the preprocessor on shaders after hashing them, so we 
> need to
> +    * add any extension override vars to the hash. If we don't do this 
> the
> +    * preprocessor could result in different output and we could load 
> the
> +    * wrong shader.
> +    */
> +   char *ext_override = getenv("MESA_EXTENSION_OVERRIDE");
> +   if (ext_override) {
> +      ralloc_asprintf_append(&buf, "ext:%s", ext_override);
> +   }
> +
>     /* DRI config options may also change the output from the compiler 
> so
>      * include them as an input to sha1 creation.
>      */
>     char sha1buf[41];
>     _mesa_sha1_format(sha1buf, ctx->Const.dri_config_options_sha1);
>     ralloc_strcat(&buf, sha1buf);
> 
>     for (unsigned i = 0; i < prog->NumShaders; i++) {
>        struct gl_shader *sh = prog->Shaders[i];
>        _mesa_sha1_format(sha1buf, sh->sha1);