[Mesa-dev] [PATCH 1/3] util/disk_cache: hash timestamps into the cache keys

Nicolai Hähnle nhaehnle at gmail.com
Tue Mar 21 08:23:29 UTC 2017


On 21.03.2017 06:14, Timothy Arceri wrote:
> From: Grazvydas Ignotas <notasas at gmail.com>
>
> Instead of using a directory, hash the timestamps into the cache keys
> themselves. Since there is no more timestamp directory, there is no more
> need for deleting the cache of other mesa versions and we rely on
> eviction to clean up the old cache entries. This solves the problem of
> using several incarnations of disk_cache at the same time, where one
> deletes a directory belonging to the other, like when both OpenGL and
> gallium nine are used simultaneously (or several different mesa
> installations).
>
> v2: using additional blob instead of trying to clone sha1 state
>
> v3: (Timothy Arceri) don't use an opaque data type to store
>     timestamp.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100091
> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
> ---
>  src/compiler/glsl/tests/cache_test.c | 10 +++---
>  src/util/disk_cache.c                | 70 ++++++++++--------------------------
>  2 files changed, 24 insertions(+), 56 deletions(-)
>
> diff --git a/src/compiler/glsl/tests/cache_test.c b/src/compiler/glsl/tests/cache_test.c
> index b1b3c33..b604943 100644
> --- a/src/compiler/glsl/tests/cache_test.c
> +++ b/src/compiler/glsl/tests/cache_test.c
> @@ -121,21 +121,21 @@ remove_entry(const char *path,
>  static int
>  rmrf_local(const char *path)
>  {
>     if (path == NULL || *path == '\0' || *path != '.')
>        return -1;
>
>     return nftw(path, remove_entry, 64, FTW_DEPTH | FTW_PHYS | FTW_MOUNT);
>  }
>
>  static void
> -check_timestamp_and_gpu_id_directories_created(char *cache_dir)
> +check_directories_created(char *cache_dir)
>  {
>     bool sub_dirs_created = false;
>
>     char buf[PATH_MAX];
>     if (getcwd(buf, PATH_MAX)) {
>        char *full_path = NULL;
>        if (asprintf(&full_path, "%s%s", buf, ++cache_dir) != -1 ) {
>           struct stat sb;
>           if (stat(full_path, &sb) != -1 && S_ISDIR(sb.st_mode))
>              sub_dirs_created = true;
> @@ -177,48 +177,48 @@ test_disk_cache_create(void)
>
>     /* Test with XDG_CACHE_HOME set */
>     setenv("XDG_CACHE_HOME", CACHE_TEST_TMP "/xdg-cache-home", 1);
>     cache = disk_cache_create("test", "make_check");
>     expect_null(cache, "disk_cache_create with XDG_CACHE_HOME set with"
>                 "a non-existing parent directory");
>
>     /* Create string with expected directory hierarchy */
>     char expected_dir_h[255];
>     sprintf(expected_dir_h, "%s%s%s", CACHE_TEST_TMP "/xdg-cache-home/mesa/",
> -           get_arch_bitness_str(), "/make_check/test");
> +           get_arch_bitness_str(), "/test");
>
>     mkdir(CACHE_TEST_TMP, 0755);
>     cache = disk_cache_create("test", "make_check");
>     expect_non_null(cache, "disk_cache_create with XDG_CACHE_HOME set");
>
> -   check_timestamp_and_gpu_id_directories_created(expected_dir_h);
> +   check_directories_created(expected_dir_h);
>
>     disk_cache_destroy(cache);
>
>     /* Test with MESA_GLSL_CACHE_DIR set */
>     err = rmrf_local(CACHE_TEST_TMP);
>     expect_equal(err, 0, "Removing " CACHE_TEST_TMP);
>
>     setenv("MESA_GLSL_CACHE_DIR", CACHE_TEST_TMP "/mesa-glsl-cache-dir", 1);
>     cache = disk_cache_create("test", "make_check");
>     expect_null(cache, "disk_cache_create with MESA_GLSL_CACHE_DIR set with"
>                 "a non-existing parent directory");
>
>     sprintf(expected_dir_h, "%s%s%s", CACHE_TEST_TMP
>             "/mesa-glsl-cache-dir/mesa/", get_arch_bitness_str(),
> -           "/make_check/test");
> +           "/test");
>
>     mkdir(CACHE_TEST_TMP, 0755);
>     cache = disk_cache_create("test", "make_check");
>     expect_non_null(cache, "disk_cache_create with MESA_GLSL_CACHE_DIR set");
>
> -   check_timestamp_and_gpu_id_directories_created(expected_dir_h);
> +   check_directories_created(expected_dir_h);
>
>     disk_cache_destroy(cache);
>  }
>
>  static bool
>  does_cache_contain(struct disk_cache *cache, cache_key key)
>  {
>     void *result;
>
>     result = disk_cache_get(cache, key, NULL);
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index dd3cadb..eb17206 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -71,20 +71,24 @@ struct disk_cache {
>     size_t index_mmap_size;
>
>     /* Pointer to total size of all objects in cache (within index_mmap) */
>     uint64_t *size;
>
>     /* Pointer to stored keys, (within index_mmap). */
>     uint8_t *stored_keys;
>
>     /* Maximum size of all cached objects (in bytes). */
>     uint64_t max_size;
> +
> +   /* Driver cache keys. */
> +   char *timestamp;
> +   size_t timestamp_size;
>  };
>
>  struct disk_cache_put_job {
>     struct util_queue_fence fence;
>
>     struct disk_cache *cache;
>
>     cache_key key;
>
>     /* Copy of cache data to be compressed and written. */
> @@ -147,80 +151,36 @@ concatenate_and_mkdir(void *ctx, const char *path, const char *name)
>        return NULL;
>
>     new_path = ralloc_asprintf(ctx, "%s/%s", path, name);
>
>     if (mkdir_if_needed(new_path) == 0)
>        return new_path;
>     else
>        return NULL;
>  }
>
> -static int
> -remove_dir(const char *fpath, const struct stat *sb,
> -           int typeflag, struct FTW *ftwbuf)
> -{
> -   if (S_ISREG(sb->st_mode))
> -      unlink(fpath);
> -   else if (S_ISDIR(sb->st_mode))
> -      rmdir(fpath);
> -
> -   return 0;
> -}
> -
> -static void
> -remove_old_cache_directories(void *mem_ctx, const char *path,
> -                             const char *timestamp)
> -{
> -   DIR *dir = opendir(path);
> -
> -   struct dirent* d_entry;
> -   while((d_entry = readdir(dir)) != NULL)
> -   {
> -      char *full_path =
> -         ralloc_asprintf(mem_ctx, "%s/%s", path, d_entry->d_name);
> -
> -      struct stat sb;
> -      if (stat(full_path, &sb) == 0 && S_ISDIR(sb.st_mode) &&
> -          strcmp(d_entry->d_name, timestamp) != 0 &&
> -          strcmp(d_entry->d_name, "..") != 0 &&
> -          strcmp(d_entry->d_name, ".") != 0) {
> -         nftw(full_path, remove_dir, 20, FTW_DEPTH);
> -      }
> -   }
> -
> -   closedir(dir);
> -}
> -
>  static char *
> -create_mesa_cache_dir(void *mem_ctx, const char *path, const char *timestamp,
> -                      const char *gpu_name)
> +create_mesa_cache_dir(void *mem_ctx, const char *path, const char *gpu_name)
>  {
>     char *new_path = concatenate_and_mkdir(mem_ctx, path, "mesa");
>     if (new_path == NULL)
>        return NULL;
>
>     /* Create a parent architecture directory so that we don't remove cache
>      * files for other architectures. In theory we could share the cache
>      * between architectures but we have no way of knowing if they were created
>      * by a compatible Mesa version.
>      */
>     new_path = concatenate_and_mkdir(mem_ctx, new_path, get_arch_bitness_str());
>     if (new_path == NULL)
>        return NULL;
>
> -   /* Remove cache directories for old Mesa versions */
> -   remove_old_cache_directories(mem_ctx, new_path, timestamp);
> -
> -   new_path = concatenate_and_mkdir(mem_ctx, new_path, timestamp);
> -   if (new_path == NULL)
> -      return NULL;
> -
>     new_path = concatenate_and_mkdir(mem_ctx, new_path, gpu_name);
>     if (new_path == NULL)
>        return NULL;
>
>     return new_path;
>  }
>
>  struct disk_cache *
>  disk_cache_create(const char *gpu_name, const char *timestamp)
>  {
> @@ -250,35 +210,33 @@ disk_cache_create(const char *gpu_name, const char *timestamp)
>      *
>      *   $MESA_GLSL_CACHE_DIR
>      *   $XDG_CACHE_HOME/mesa
>      *   <pwd.pw_dir>/.cache/mesa
>      */
>     path = getenv("MESA_GLSL_CACHE_DIR");
>     if (path) {
>        if (mkdir_if_needed(path) == -1)
>           goto fail;
>
> -      path = create_mesa_cache_dir(local, path, timestamp,
> -                                   gpu_name);
> +      path = create_mesa_cache_dir(local, path, gpu_name);
>        if (path == NULL)
>           goto fail;
>     }
>
>     if (path == NULL) {
>        char *xdg_cache_home = getenv("XDG_CACHE_HOME");
>
>        if (xdg_cache_home) {
>           if (mkdir_if_needed(xdg_cache_home) == -1)
>              goto fail;
>
> -         path = create_mesa_cache_dir(local, xdg_cache_home, timestamp,
> -                                      gpu_name);
> +         path = create_mesa_cache_dir(local, xdg_cache_home, gpu_name);
>           if (path == NULL)
>              goto fail;
>        }
>     }
>
>     if (path == NULL) {
>        char *buf;
>        size_t buf_size;
>        struct passwd pwd, *result;
>
> @@ -300,29 +258,34 @@ disk_cache_create(const char *gpu_name, const char *timestamp)
>              buf_size *= 2;
>           } else {
>              goto fail;
>           }
>        }
>
>        path = concatenate_and_mkdir(local, pwd.pw_dir, ".cache");
>        if (path == NULL)
>           goto fail;
>
> -      path = create_mesa_cache_dir(local, path, timestamp, gpu_name);
> +      path = create_mesa_cache_dir(local, path, gpu_name);
>        if (path == NULL)
>           goto fail;
>     }
>
>     cache = ralloc(NULL, struct disk_cache);
>     if (cache == NULL)
>        goto fail;
>
> +   cache->timestamp_size = strlen(timestamp);
> +   cache->timestamp = ralloc_strdup(cache,  timestamp);
> +   if (cache->timestamp == NULL)
> +      goto fail;
> +
>     cache->path = ralloc_strdup(cache, path);
>     if (cache->path == NULL)
>        goto fail;
>
>     path = ralloc_asprintf(local, "%s/index", cache->path);
>     if (path == NULL)
>        goto fail;
>
>     fd = open(path, O_RDWR | O_CREAT | O_CLOEXEC, 0644);
>     if (fd == -1)
> @@ -1069,14 +1032,19 @@ disk_cache_has_key(struct disk_cache *cache, const cache_key key)
>
>     entry = &cache->stored_keys[i * CACHE_KEY_SIZE];
>
>     return memcmp(entry, key, CACHE_KEY_SIZE) == 0;
>  }
>
>  void
>  disk_cache_compute_key(struct disk_cache *cache, const void *data, size_t size,
>                         cache_key key)
>  {
> -   _mesa_sha1_compute(data, size, key);
> +   struct mesa_sha1 ctx;
> +
> +   _mesa_sha1_init(&ctx);
> +   _mesa_sha1_update(&ctx, cache->timestamp, cache->timestamp_size);

I'd suggest cache->timestamp_size + 1 here for extra paranoia: by adding 
the final null, you avoid conflicts in the extremely unlikely case where 
the start of the data corresponds to the suffix of some timestamp string.

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



> +   _mesa_sha1_update(&ctx, data, size);
> +   _mesa_sha1_final(&ctx, key);
>  }
>
>  #endif /* ENABLE_SHADER_CACHE */
>



More information about the mesa-dev mailing list