On 24/02/17 08:49, Timothy Arceri wrote:


On 24/02/17 05:12, Marek Olšák wrote:
On Thu, Feb 23, 2017 at 3:09 AM, Timothy Arceri
<[email protected]> wrote:
From: kdj0c <[email protected]>

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.


For some reason I thought the key was simpler than it is. It seems excessive to store and compare the tgsi again. I don't think git even worries about the possibility of a collision and we will be dealing with much smaller amounts of cache items then commits in a git repository.

Thoughts?



+               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

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to