[Mesa-dev] [PATCH 3/5] disk cache: initialize cache path and index only when used

[Tapani Pälli]

Reviewing own code ...

This change does not work well with existing unit tests for disk cache. 
I'll have to do some changes to those tests.

On 01/22/2018 03:00 PM, Tapani Pälli wrote:
> This patch makes disk_cache initialize path and index lazily so
> that we can utilize disk_cache without a path using callback
> functionality introduced by next patch.
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>   src/util/disk_cache.c | 114 ++++++++++++++++++++++++++++++++------------------
>   1 file changed, 73 insertions(+), 41 deletions(-)
> 
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index 2884d3c9c1..2fb07699b5 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -77,6 +77,7 @@
>   struct disk_cache {
>      /* The path to the cache directory. */
>      char *path;
> +   bool path_init_failed;
>   
>      /* Thread queue for compressing and writing cache entries to disk */
>      struct util_queue cache_queue;
> @@ -178,27 +179,18 @@ concatenate_and_mkdir(void *ctx, const char *path, const char *name)
>         return NULL;
>   }
>   
> -#define DRV_KEY_CPY(_dst, _src, _src_size) \
> -do {                                       \
> -   memcpy(_dst, _src, _src_size);          \
> -   _dst += _src_size;                      \
> -} while (0);
> -
> -struct disk_cache *
> -disk_cache_create(const char *gpu_name, const char *timestamp,
> -                  uint64_t driver_flags)
> +static bool
> +disk_cache_path_init(struct disk_cache *cache)
>   {
> -   void *local;
> -   struct disk_cache *cache = NULL;
> -   char *path, *max_size_str;
> -   uint64_t max_size;
> +   void *local = NULL;
> +   char *path;
>      int fd = -1;
>      struct stat sb;
>      size_t size;
>   
>      /* If running as a users other than the real user disable cache */
>      if (geteuid() != getuid())
> -      return NULL;
> +      goto fail;
>   
>      /* A ralloc context for transient data during this invocation. */
>      local = ralloc_context(NULL);
> @@ -273,10 +265,6 @@ disk_cache_create(const char *gpu_name, const char *timestamp,
>            goto fail;
>      }
>   
> -   cache = ralloc(NULL, struct disk_cache);
> -   if (cache == NULL)
> -      goto fail;
> -
>      cache->path = ralloc_strdup(cache, path);
>      if (cache->path == NULL)
>         goto fail;
> @@ -325,6 +313,58 @@ disk_cache_create(const char *gpu_name, const char *timestamp,
>      cache->size = (uint64_t *) cache->index_mmap;
>      cache->stored_keys = cache->index_mmap + sizeof(uint64_t);
>   
> +   /* 1 thread was chosen because we don't really care about getting things
> +    * to disk quickly just that it's not blocking other tasks.
> +    *
> +    * The queue will resize automatically when it's full, so adding new jobs
> +    * doesn't stall.
> +    */
> +   util_queue_init(&cache->cache_queue, "disk_cache", 32, 1,
> +                   UTIL_QUEUE_INIT_RESIZE_IF_FULL |
> +                   UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY);
> +
> +   ralloc_free(local);
> +
> +   return true;
> +
> + fail:
> +   if (fd != -1)
> +      close(fd);
> +
> +   if (local)
> +      ralloc_free(local);
> +
> +   cache->path_init_failed = true;
> +
> +   return false;
> +}
> +
> +#define DRV_KEY_CPY(_dst, _src, _src_size) \
> +do {                                       \
> +   memcpy(_dst, _src, _src_size);          \
> +   _dst += _src_size;                      \
> +} while (0);
> +
> +struct disk_cache *
> +disk_cache_create(const char *gpu_name, const char *timestamp,
> +                  uint64_t driver_flags)
> +{
> +   struct disk_cache *cache = NULL;
> +   char *max_size_str;
> +   uint64_t max_size;
> +
> +   /* If running as a users other than the real user disable cache */
> +   if (geteuid() != getuid())
> +      return NULL;
> +
> +   /* At user request, disable shader cache entirely. */
> +   if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", false))
> +      return NULL;
> +
> +   cache = rzalloc(NULL, struct disk_cache);
> +   if (cache == NULL)
> +      return NULL;
> +
>      max_size = 0;
>   
>      max_size_str = getenv("MESA_GLSL_CACHE_MAX_SIZE");
> @@ -360,16 +400,6 @@ disk_cache_create(const char *gpu_name, const char *timestamp,
>   
>      cache->max_size = max_size;
>   
> -   /* 1 thread was chosen because we don't really care about getting things
> -    * to disk quickly just that it's not blocking other tasks.
> -    *
> -    * The queue will resize automatically when it's full, so adding new jobs
> -    * doesn't stall.
> -    */
> -   util_queue_init(&cache->cache_queue, "disk_cache", 32, 1,
> -                   UTIL_QUEUE_INIT_RESIZE_IF_FULL |
> -                   UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY);
> -
>      uint8_t cache_version = CACHE_VERSION;
>      size_t cv_size = sizeof(cache_version);
>      cache->driver_keys_blob_size = cv_size;
> @@ -392,8 +422,10 @@ disk_cache_create(const char *gpu_name, const char *timestamp,
>   
>      cache->driver_keys_blob =
>         ralloc_size(cache, cache->driver_keys_blob_size);
> -   if (!cache->driver_keys_blob)
> -      goto fail;
> +   if (!cache->driver_keys_blob) {
> +      ralloc_free(cache);
> +      return NULL;
> +   }
>   
>      uint8_t *drv_key_blob = cache->driver_keys_blob;
>      DRV_KEY_CPY(drv_key_blob, &cache_version, cv_size)
> @@ -405,18 +437,7 @@ disk_cache_create(const char *gpu_name, const char *timestamp,
>      /* Seed our rand function */
>      s_rand_xorshift128plus(cache->seed_xorshift128plus, true);
>   
> -   ralloc_free(local);
> -
>      return cache;
> -
> - fail:
> -   if (fd != -1)
> -      close(fd);
> -   if (cache)
> -      ralloc_free(cache);
> -   ralloc_free(local);
> -
> -   return NULL;
>   }
>   
>   void
> @@ -441,6 +462,9 @@ get_cache_file(struct disk_cache *cache, const cache_key key)
>      char buf[41];
>      char *filename;
>   
> +   if (!cache->path)
> +      return NULL;
> +
>      _mesa_sha1_format(buf, key);
>      if (asprintf(&filename, "%s/%c%c/%s", cache->path, buf[0],
>                   buf[1], buf + 2) == -1)
> @@ -1173,6 +1197,9 @@ disk_cache_put_key(struct disk_cache *cache, const cache_key key)
>      int i = CPU_TO_LE32(*key_chunk) & CACHE_INDEX_KEY_MASK;
>      unsigned char *entry;
>   
> +   if (!cache->path)
> +      return;
> +
>      entry = &cache->stored_keys[i * CACHE_KEY_SIZE];
>   
>      memcpy(entry, key, CACHE_KEY_SIZE);
> @@ -1192,6 +1219,11 @@ disk_cache_has_key(struct disk_cache *cache, const cache_key key)
>      int i = CPU_TO_LE32(*key_chunk) & CACHE_INDEX_KEY_MASK;
>      unsigned char *entry;
>   
> +   /* Try initializing path only once. */
> +   if (cache->path_init_failed ||
> +       (!cache->path && !disk_cache_path_init(cache)))
> +      return false;
> +
>      entry = &cache->stored_keys[i * CACHE_KEY_SIZE];
>   
>      return memcmp(entry, key, CACHE_KEY_SIZE) == 0;
>