[Mesa-dev] [PATCH 2/3] util/disk_cache: hash pointer size and gpu name into cache keys
Nicolai Hähnle
nhaehnle at gmail.com
Tue Mar 21 08:25:17 UTC 2017
On 21.03.2017 06:14, Timothy Arceri wrote:
> From: Grazvydas Ignotas <notasas at gmail.com>
>
> This allows to get rid of the arch and gpu name directories.
>
> v2: (Timothy Arceri) don't use an opaque data type to store
> pointer size and gpu name.
>
> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
> ---
> src/compiler/glsl/tests/cache_test.c | 13 ++---------
> src/util/disk_cache.c | 44 +++++++++++++++---------------------
> src/util/disk_cache.h | 17 --------------
> 3 files changed, 20 insertions(+), 54 deletions(-)
>
> diff --git a/src/compiler/glsl/tests/cache_test.c b/src/compiler/glsl/tests/cache_test.c
> index b604943..2302f44 100644
> --- a/src/compiler/glsl/tests/cache_test.c
> +++ b/src/compiler/glsl/tests/cache_test.c
> @@ -174,51 +174,42 @@ test_disk_cache_create(void)
> expect_non_null(cache, "disk_cache_create with no environment variables");
>
> disk_cache_destroy(cache);
>
> /* 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(), "/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_directories_created(expected_dir_h);
> + check_directories_created(CACHE_TEST_TMP "/xdg-cache-home/mesa");
>
> 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(),
> - "/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_directories_created(expected_dir_h);
> + check_directories_created(CACHE_TEST_TMP "/mesa-glsl-cache-dir/mesa");
>
> 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 eb17206..599e58f 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -75,20 +75,23 @@ struct disk_cache {
>
> /* 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;
> + char *gpu_name;
> + size_t gpu_name_size;
> + uint8_t ptr_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. */
> @@ -151,43 +154,20 @@ 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 char *
> -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;
> -
> - 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)
> {
> void *local;
> struct disk_cache *cache = NULL;
> char *path, *max_size_str;
> uint64_t max_size;
> int fd = -1;
> struct stat sb;
> struct statvfs vfs = { 0 };
> @@ -210,33 +190,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, gpu_name);
> + path = concatenate_and_mkdir(local, path, "mesa");
> 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, gpu_name);
> + path = concatenate_and_mkdir(local, xdg_cache_home, "mesa");
> if (path == NULL)
> goto fail;
> }
> }
>
> if (path == NULL) {
> char *buf;
> size_t buf_size;
> struct passwd pwd, *result;
>
> @@ -258,34 +238,44 @@ 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, gpu_name);
> + path = concatenate_and_mkdir(local, path, "mesa");
> 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->gpu_name_size = strlen(gpu_name);
> + cache->gpu_name = ralloc_strdup(cache, gpu_name);
> + if (cache->gpu_name == NULL)
> + goto fail;
> +
> + /* We sometimes store entire structs that contains a pointers in the cache,
> + * use pointer size as a key to avoid hard to debug issues.
> + */
> + cache->ptr_size = sizeof(void *);
> +
> 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)
> @@ -1036,15 +1026,17 @@ disk_cache_has_key(struct disk_cache *cache, const cache_key key)
> }
>
> void
> disk_cache_compute_key(struct disk_cache *cache, const void *data, size_t size,
> cache_key key)
> {
> struct mesa_sha1 ctx;
>
> _mesa_sha1_init(&ctx);
> _mesa_sha1_update(&ctx, cache->timestamp, cache->timestamp_size);
> + _mesa_sha1_update(&ctx, cache->gpu_name, cache->gpu_name_size);
For consistency, please add the same + 1 as in the previous patch.
> + _mesa_sha1_update(&ctx, &cache->ptr_size, sizeof(uint8_t));
sizeof(cache->ptr_size) seems more robust to me.
With those 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 */
> diff --git a/src/util/disk_cache.h b/src/util/disk_cache.h
> index 965fea1..2bb1cf5 100644
> --- a/src/util/disk_cache.h
> +++ b/src/util/disk_cache.h
> @@ -36,37 +36,20 @@
> extern "C" {
> #endif
>
> /* Size of cache keys in bytes. */
> #define CACHE_KEY_SIZE 20
>
> typedef uint8_t cache_key[CACHE_KEY_SIZE];
>
> struct disk_cache;
>
> -static inline const char *
> -get_arch_bitness_str(void)
> -{
> - if (sizeof(void *) == 4)
> -#ifdef __ILP32__
> - return "ilp-32";
> -#else
> - return "32";
> -#endif
> - if (sizeof(void *) == 8)
> - return "64";
> -
> - /* paranoia check which will be dropped by the optimiser */
> - assert(!"unknown_arch");
> - return "unknown_arch";
> -}
> -
> static inline bool
> disk_cache_get_function_timestamp(void *ptr, uint32_t* timestamp)
> {
> #ifdef ENABLE_SHADER_CACHE
> Dl_info info;
> struct stat st;
> if (!dladdr(ptr, &info) || !info.dli_fname) {
> return false;
> }
> if (stat(info.dli_fname, &st)) {
>
More information about the mesa-dev
mailing list