[Mesa-dev] [PATCH 2/3] util/disk_cache: hash pointer size and gpu name into cache keys
Timothy Arceri
tarceri at itsqueeze.com
Wed Mar 22 02:44:45 UTC 2017
On 21/03/17 21:23, Timothy Arceri wrote:
>
>
> On 21/03/17 21:01, Nicolai Hähnle wrote:
>> On 21.03.2017 10:22, Timothy Arceri wrote:
>>>
>>>
>>> On 21/03/17 19:25, Nicolai Hähnle wrote:
>>>> 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.
>>>
>>>
>>> I was trying to appease the people complaining about weakening the hash
>>> by salting it with this driver identifying data.
>>
>> Huh, I guess I missed that discussion due to vacation and massive
>> backlog, but I don't see why that would weaken anything. Do you have a
>> link?
>
> I don't think it would make much difference either but I'm sick of all
> the "you should do it this way" shader cache discussions.
Just to be clean I meant from the peanut gallery, not legitimate
discussions/reviews.
More information about the mesa-dev
mailing list