[Mesa-dev] [PATCH V3 2/3] util/disk_cache: hash pointer size and gpu name into cache keys
Timothy Arceri
tarceri at itsqueeze.com
Wed Mar 22 07:34:09 UTC 2017
On 22/03/17 18:17, Nicolai Hähnle wrote:
> On 22.03.2017 04:45, 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.
>>
>> v3: (Timothy Arceri) use blob to store driver keys just make sure
>> to store null terminator for strings, and make sure blob is
>> defined by disk_cache and not it's users.
>>
>> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
>> ---
>> src/compiler/glsl/tests/cache_test.c | 13 ++----------
>> src/util/disk_cache.c | 41
>> +++++++++++++-----------------------
>> src/util/disk_cache.h | 17 ---------------
>> 3 files changed, 17 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 c9c0492..8fb59be 100644
>> --- a/src/util/disk_cache.c
>> +++ b/src/util/disk_cache.c
>> @@ -151,43 +151,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 +187,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,21 +235,21 @@ 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->path = ralloc_strdup(cache, path);
>> if (cache->path == NULL)
>> @@ -361,28 +338,40 @@ disk_cache_create(const char *gpu_name, const
>> char *timestamp)
>> /* A limit of 32 jobs was choosen as observations of Deus Ex
>> start-up times
>> * showed that we reached at most 11 jobs on an Intel i5-6400
>> CPU at 2.70GHz
>> * (a fairly modest desktop CPU). 1 thread was chosen because we
>> don't
>> * really care about getting things to disk quickly just that it's
>> not
>> * blocking other tasks.
>> */
>> util_queue_init(&cache->cache_queue, "disk_cache", 32, 1);
>>
>> /* Create driver id keys */
>> size_t ts_size = strlen(timestamp) + 1;
>> + size_t gpu_name_size = strlen(gpu_name) + 1;
>> cache->driver_keys_blob_size = ts_size;
>> + cache->driver_keys_blob_size += gpu_name_size;
>> +
>> + /* We sometimes store entire structs that contains a pointers in
>> the cache,
>> + * use pointer size as a key to avoid hard to debug issues.
>> + */
>> + size_t uinit32_size = sizeof(uint32_t);
>
> Typo: uint32
>
>
>> + size_t ptr_size = sizeof(void *);
>> + cache->driver_keys_blob_size += uinit32_size;
>>
>> cache->driver_keys_blob =
>> ralloc_size(cache, cache->driver_keys_blob_size);
>> if (!cache->driver_keys_blob)
>> goto fail;
>>
>> memcpy(cache->driver_keys_blob, timestamp, ts_size);
>> + memcpy(cache->driver_keys_blob + ts_size, gpu_name, gpu_name_size);
>> + memcpy(cache->driver_keys_blob + ts_size + gpu_name_size, &ptr_size,
>> + uinit32_size);
>
> This is broken in the big-endian case. uint32_size should be
> sizeof(ptr_size).
The idea is we don't want a variable size as apps should be able to
parse these from the header. I guess I probably want to copy ptr_size to
a uint32_t or even just uint8_t before the memcpy.
>
> Cheers,
> Nicolai
>
>>
>> ralloc_free(local);
>>
>> return cache;
>>
>> fail:
>> if (fd != -1)
>> close(fd);
>> if (cache)
>> ralloc_free(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