[Mesa-dev] [PATCH v2 5/5] util/disk_cache: write cache item metadata to disk

[Nicolai Hähnle]

On 23.08.2017 08:33, Timothy Arceri wrote:
> ---
>   src/util/disk_cache.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index c1b7429b92..dd12a09841 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -105,20 +105,22 @@ struct disk_cache_put_job {
>   
>      struct disk_cache *cache;
>   
>      cache_key key;
>   
>      /* Copy of cache data to be compressed and written. */
>      void *data;
>   
>      /* Size of data to be compressed and written. */
>      size_t size;
> +
> +   struct cache_item_metadata cache_item_metadata;
>   };
>   
>   /* Create a directory named 'path' if it does not already exist.
>    *
>    * Returns: 0 if path already exists as a directory or if created.
>    *         -1 in all other cases.
>    */
>   static int
>   mkdir_if_needed(const char *path)
>   {
> @@ -771,29 +773,60 @@ create_put_job(struct disk_cache *cache, const cache_key key,
>   {
>      struct disk_cache_put_job *dc_job = (struct disk_cache_put_job *)
>         malloc(sizeof(struct disk_cache_put_job) + size);
>   
>      if (dc_job) {
>         dc_job->cache = cache;
>         memcpy(dc_job->key, key, sizeof(cache_key));
>         dc_job->data = dc_job + 1;
>         memcpy(dc_job->data, data, size);
>         dc_job->size = size;
> +
> +      /* Copy the cache item metadata */
> +      if (cache_item_metadata) {
> +         dc_job->cache_item_metadata.type = cache_item_metadata->type;
> +         if (cache_item_metadata->type == CACHE_ITEM_TYPE_GLSL) {
> +            dc_job->cache_item_metadata.num_keys =
> +               cache_item_metadata->num_keys;
> +            dc_job->cache_item_metadata.keys = (cache_key *)
> +               malloc(cache_item_metadata->num_keys * sizeof(cache_key));
> +
> +            if (!dc_job->cache_item_metadata.keys)
> +               goto fail;
> +
> +            for (unsigned i = 0; i < cache_item_metadata->num_keys; i++) {
> +               memcpy(dc_job->cache_item_metadata.keys[i],
> +                      cache_item_metadata->keys[i], sizeof(cache_key));
> +            }

You can just use a single memcpy.

With that fixed, patches 2-5 are

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>


> +         }
> +      } else {
> +         dc_job->cache_item_metadata.type = CACHE_ITEM_TYPE_UNKNOWN;
> +         dc_job->cache_item_metadata.keys = NULL;
> +      }
>      }
>   
>      return dc_job;
> +
> +fail:
> +   free(dc_job->cache_item_metadata.keys);
> +   free(dc_job);
> +
> +   return NULL;
>   }
>   
>   static void
>   destroy_put_job(void *job, int thread_index)
>   {
>      if (job) {
> +      struct disk_cache_put_job *dc_job = (struct disk_cache_put_job *) job;
> +      free(dc_job->cache_item_metadata.keys);
> +
>         free(job);
>      }
>   }
>   
>   struct cache_entry_file_data {
>      uint32_t crc32;
>      uint32_t uncompressed_size;
>   };
>   
>   static void
> @@ -868,20 +901,48 @@ cache_put(void *job, int thread_index)
>       * mesa version that produced the entry or deal with hash collisions,
>       * should that ever become a real problem.
>       */
>      ret = write_all(fd, dc_job->cache->driver_keys_blob,
>                      dc_job->cache->driver_keys_blob_size);
>      if (ret == -1) {
>         unlink(filename_tmp);
>         goto done;
>      }
>   
> +   /* Write the cache item metadata. This data can be used to deal with
> +    * hash collisions, as well as providing useful information to 3rd party
> +    * tools reading the cache files.
> +    */
> +   ret = write_all(fd, &dc_job->cache_item_metadata.type,
> +                   sizeof(uint32_t));
> +   if (ret == -1) {
> +      unlink(filename_tmp);
> +      goto done;
> +   }
> +
> +   if (dc_job->cache_item_metadata.type == CACHE_ITEM_TYPE_GLSL) {
> +      ret = write_all(fd, &dc_job->cache_item_metadata.num_keys,
> +                      sizeof(uint32_t));
> +      if (ret == -1) {
> +         unlink(filename_tmp);
> +         goto done;
> +      }
> +
> +      ret = write_all(fd, dc_job->cache_item_metadata.keys[0],
> +                      dc_job->cache_item_metadata.num_keys *
> +                      sizeof(cache_key));
> +      if (ret == -1) {
> +         unlink(filename_tmp);
> +         goto done;
> +      }
> +   }
> +
>      /* Create CRC of the data. We will read this when restoring the cache and
>       * use it to check for corruption.
>       */
>      struct cache_entry_file_data cf_data;
>      cf_data.crc32 = util_hash_crc32(dc_job->data, dc_job->size);
>      cf_data.uncompressed_size = dc_job->size;
>   
>      size_t cf_data_size = sizeof(cf_data);
>      ret = write_all(fd, &cf_data, cf_data_size);
>      if (ret == -1) {
> @@ -1019,29 +1080,55 @@ disk_cache_get(struct disk_cache *cache, const cache_key key, size_t *size)
>         goto fail;
>   
>      ret = read_all(fd, file_header, ck_size);
>      if (ret == -1)
>         goto fail;
>   
>      /* Check for extremely unlikely hash collisions */
>      if (memcmp(cache->driver_keys_blob, file_header, ck_size) != 0)
>         goto fail;
>   
> +   size_t cache_item_md_size = sizeof(uint32_t);
> +   uint32_t md_type;
> +   ret = read_all(fd, &md_type, cache_item_md_size);
> +   if (ret == -1)
> +      goto fail;
> +
> +   if (md_type == CACHE_ITEM_TYPE_GLSL) {
> +      uint32_t num_keys;
> +      cache_item_md_size += sizeof(uint32_t);
> +      ret = read_all(fd, &num_keys, sizeof(uint32_t));
> +      if (ret == -1)
> +         goto fail;
> +
> +      /* The cache item metadata is currently just used for distributing
> +       * precompiled shaders, they are not used by Mesa so just skip them for
> +       * now.
> +       * TODO: pass the metadata back to the caller and do some basic
> +       * validation.
> +       */
> +      cache_item_md_size += sizeof(cache_key);
> +      ret = lseek(fd, num_keys * sizeof(cache_key), SEEK_CUR);
> +      if (ret == -1)
> +         goto fail;
> +   }
> +
>      /* Load the CRC that was created when the file was written. */
>      struct cache_entry_file_data cf_data;
>      size_t cf_data_size = sizeof(cf_data);
>      ret = read_all(fd, &cf_data, cf_data_size);
>      if (ret == -1)
>         goto fail;
>   
>      /* Load the actual cache data. */
> -   size_t cache_data_size = sb.st_size - cf_data_size - ck_size;
> +   size_t cache_data_size =
> +      sb.st_size - cf_data_size - ck_size - cache_item_md_size;
>      ret = read_all(fd, data, cache_data_size);
>      if (ret == -1)
>         goto fail;
>   
>      /* Uncompress the cache data */
>      uncompressed_data = malloc(cf_data.uncompressed_size);
>      if (!inflate_cache_data(data, cache_data_size, uncompressed_data,
>                              cf_data.uncompressed_size))
>         goto fail;
>   
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.