[Mesa-dev] [PATCH V3 2/3] util/disk_cache: hash pointer size and gpu name into cache keys
Nicolai Hähnle
nhaehnle at gmail.com
Wed Mar 22 07:46:11 UTC 2017
On 22.03.2017 08:34, Timothy Arceri wrote:
>
>
> 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.
Yes, that would work fine.
Nicolai
>
>>
>> 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