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

[Timothy Arceri]

On 27/03/17 08:44, Timothy Arceri wrote:
> 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.

Although that won't work for shaders that fail to compile. So I guess 
disabling optimistaions might be the only option.

Anyway I'll push this change. Thanks!

>
> 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;
>>  }
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev