[Mesa-dev] [PATCH 1/2] radeonsi: add support for an on-disk shader cache

[Timothy Arceri]

On 24/02/17 05:12, Marek Olšák wrote:
> On Thu, Feb 23, 2017 at 3:09 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> From: kdj0c <kdj0c at djinvi.net>
>>
>> V2 (Timothy Arceri):
>> - when loading from disk cache also binary insert into memory cache.
>> - check that the binary loaded from disk is the correct size. If not
>>   delete the cache item and skip loading from cache.
>> ---
>>  src/gallium/drivers/radeonsi/si_state_shaders.c | 69 ++++++++++++++++++++++---
>>  1 file changed, 62 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> index f615aa8..71556f9 100644
>> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
>> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> @@ -36,6 +36,9 @@
>>  #include "util/u_memory.h"
>>  #include "util/u_prim.h"
>>
>> +#include "util/disk_cache.h"
>> +#include "util/mesa-sha1.h"
>> +
>>  /* SHADER_CACHE */
>>
>>  /**
>> @@ -182,10 +185,12 @@ static bool si_load_shader_binary(struct si_shader *shader, void *binary)
>>   */
>>  static bool si_shader_cache_insert_shader(struct si_screen *sscreen,
>>                                           void *tgsi_binary,
>> -                                         struct si_shader *shader)
>> +                                         struct si_shader *shader,
>> +                                         bool insert_into_disk_cache)
>>  {
>>         void *hw_binary;
>>         struct hash_entry *entry;
>> +       uint8_t key[CACHE_KEY_SIZE];
>>
>>         entry = _mesa_hash_table_search(sscreen->shader_cache, tgsi_binary);
>>         if (entry)
>> @@ -201,6 +206,12 @@ static bool si_shader_cache_insert_shader(struct si_screen *sscreen,
>>                 return false;
>>         }
>>
>> +       if (sscreen->b.disk_shader_cache && insert_into_disk_cache) {
>> +               _mesa_sha1_compute(tgsi_binary, *((uint32_t *)tgsi_binary), key);
>
> What happens if we randomly get a sha1 collision?

You should stop playing your game which will be rendering incorrectly 
and by a lotto ticket.

> Shouldn't we store the whole key as well?

Sure I can add that, its cheap to check here anyway. Although the other 
cache stages rely on a collision being improbable.

>
>> +               disk_cache_put(sscreen->b.disk_shader_cache, key, hw_binary,
>> +                              *((uint32_t *) hw_binary));
>> +       }
>> +
>>         return true;
>>  }
>>
>> @@ -210,12 +221,56 @@ static bool si_shader_cache_load_shader(struct si_screen *sscreen,
>>  {
>>         struct hash_entry *entry =
>>                 _mesa_hash_table_search(sscreen->shader_cache, tgsi_binary);
>> -       if (!entry)
>> -               return false;
>> +       if (!entry) {
>> +               if (sscreen->b.disk_shader_cache) {
>> +                       unsigned char sha1[CACHE_KEY_SIZE];
>> +                       size_t tg_size = *((uint32_t *) tgsi_binary);
>> +
>> +                       _mesa_sha1_compute(tgsi_binary, tg_size, sha1);
>> +
>> +                       size_t binary_size;
>> +                       uint8_t *buffer =
>> +                               disk_cache_get(sscreen->b.disk_shader_cache,
>> +                                              sha1, &binary_size);
>> +                       if (!buffer)
>> +                               return false;
>>
>> -       if (!si_load_shader_binary(shader, entry->data))
>> -               return false;
>> +                       uint32_t stored_binary_size;
>
> It looks like you don't need this variable.


Right, I'll tidy that up thanks.

>
> Marek
>