[Mesa-dev] [PATCH] glsl/shader_cache: restore evicted shader keys

[Timothy Arceri]

I'm actually wondering if we still even need the index at all? After 
bfa95997c4ecf74a329a047359d5e8d1217da49b and 
b607aad8e1a40c473176e31e11deaa26477c54c0 we should be able to just 
always set CompileStatus to skipped and fallback to a recompile should 
the linked program not be found in the cache.

To me the chance of having to compile the shader once on the first run 
at link time seems better than constantly recompiling shaders when we 
hit collisions. With an index table of only 65,536 entries the chance of 
collisions is fairly high even if you have only run a couple of 
applications.

The other option could be to delay shader optimisations until we have 
checked the linked program is not in the cache. That should cut down 
times when we have collisions in the index but the linked program is 
still in the cache.


On 27/03/17 03:28, Grazvydas Ignotas wrote:
> Even though the programs themselves stay in cache and are loaded, the
> shader keys can be evicted separately. If that happens, unnecessary
> compiles are caused that waste time, and no matter how many times the
> program is re-run, performance never recovers to the levels of first
> hot cache run. To deal with this, we need to refresh the shader keys
> of shaders that were recompiled.
>
> An easy way to currently observe this is running Deux Ex, then piglit
> and Deux Ex again, or deleting just the cache index. The later is
> causing over a minute of lost time on all later Deux Ex runs, with this
> patch it returns to normal after 1 run.
>
> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
> ---
>  src/compiler/glsl/shader_cache.cpp | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp
> index dd8c6c0..274bb8c 100644
> --- a/src/compiler/glsl/shader_cache.cpp
> +++ b/src/compiler/glsl/shader_cache.cpp
> @@ -1407,9 +1407,26 @@ shader_cache_read_program_metadata(struct gl_context *ctx,
>     }
>
>     /* This is used to flag a shader retrieved from cache */
>     prog->data->LinkStatus = linking_skipped;
>
> +   /* Since the program load was successful, CompileStatus of all shaders at
> +    * this point should normally be compile_skipped. However because of how
> +    * the eviction works, it may happen that some of the individual shader keys
> +    * have been evicted, resulting in unnecessary recompiles on this load, so
> +    * mark them again to skip such recompiles next time.
> +    */
> +   char sha1_buf[41];
> +   for (unsigned i = 0; i < prog->NumShaders; i++) {
> +      if (prog->Shaders[i]->CompileStatus == compile_success) {
> +         disk_cache_put_key(cache, prog->Shaders[i]->sha1);
> +         if (ctx->_Shader->Flags & GLSL_CACHE_INFO) {
> +            _mesa_sha1_format(sha1_buf, prog->Shaders[i]->sha1);
> +            fprintf(stderr, "re-marking shader: %s\n", sha1_buf);
> +         }
> +      }
> +   }
> +
>     free (buffer);
>
>     return true;
>  }
>