[Mesa-dev] [PATCH 5/7] util/disk_cache: hash pointer size and gpu name into cache keys
Grazvydas Ignotas
notasas at gmail.com
Thu Mar 16 13:43:43 UTC 2017
On Thu, Mar 16, 2017 at 3:14 PM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>
>
> On 17/03/17 00:00, Grazvydas Ignotas wrote:
>>
>> On Thu, Mar 16, 2017 at 2:49 AM, Timothy Arceri <tarceri at itsqueeze.com>
>> wrote:
>>>
>>>
>>>
>>> On 16/03/17 10:09, Grazvydas Ignotas wrote:
>>>>
>>>>
>>>> This allows to get rid of the arch and gpu name directories.
>>>> Also, gpu_name is now optional.
>>>>
>>>> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
>>>> ---
>>>> src/compiler/glsl/tests/cache_test.c | 13 ++--------
>>>> src/util/disk_cache.c | 47
>>>> +++++++++++++++---------------------
>>>> src/util/disk_cache.h | 17 -------------
>>>> 3 files changed, 21 insertions(+), 56 deletions(-)
>>>>
>>>> diff --git a/src/compiler/glsl/tests/cache_test.c
>>>> b/src/compiler/glsl/tests/cache_test.c
>>>> index f923e8d..f6d3621 100644
>>>> --- a/src/compiler/glsl/tests/cache_test.c
>>>> +++ b/src/compiler/glsl/tests/cache_test.c
>>>> @@ -182,16 +182,11 @@ test_disk_cache_create(void)
>>>> 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", version_blob,
>>>> sizeof(version_blob));
>>>> 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);
>>>>
>>>> @@ -204,15 +199,11 @@ test_disk_cache_create(void)
>>>> 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", version_blob,
>>>> sizeof(version_blob));
>>>> 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);
>>>> }
>>>> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
>>>> index 5b249d9..904aa66 100644
>>>> --- a/src/util/disk_cache.c
>>>> +++ b/src/util/disk_cache.c
>>>> @@ -158,29 +158,6 @@ concatenate_and_mkdir(void *ctx, const char *path,
>>>> const char *name)
>>>> 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 void *version_blob,
>>>> size_t version_blob_size)
>>>> @@ -218,7 +195,7 @@ disk_cache_create(const char *gpu_name, const void
>>>> *version_blob,
>>>> 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;
>>>> }
>>>> @@ -230,7 +207,7 @@ disk_cache_create(const char *gpu_name, const void
>>>> *version_blob,
>>>> 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;
>>>> }
>>>> @@ -266,7 +243,7 @@ disk_cache_create(const char *gpu_name, const void
>>>> *version_blob,
>>>> 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;
>>>> }
>>>> @@ -275,11 +252,25 @@ disk_cache_create(const char *gpu_name, const void
>>>> *version_blob,
>>>> if (cache == NULL)
>>>> goto fail;
>>>>
>>>> - cache->key_blob_size = version_blob_size;
>>>> + /* Prepare the blob to hash into the cache keys */
>>>> + uint8_t pointer_size = sizeof(void *);
>>>> + cache->key_blob_size = version_blob_size + sizeof(pointer_size);
>>>> + if (gpu_name != NULL)
>>>> + cache->key_blob_size += strlen(gpu_name);
>>>> cache->key_blob = ralloc_size(cache, cache->key_blob_size);
>>>> if (cache->key_blob == NULL)
>>>> goto fail;
>>>> - memcpy(cache->key_blob, version_blob, version_blob_size);
>>>> +
>>>> + char *pos = cache->key_blob;
>>>> + memcpy(pos, version_blob, version_blob_size);
>>>> + pos += version_blob_size;
>>>> + memcpy(pos, &pointer_size, sizeof(pointer_size));
>>>> + pos += sizeof(pointer_size);
>>>> + if (gpu_name != NULL) {
>>>
>>>
>>>
>>> Is there any valid reason for this to be NULL? Can we just add an
>>> assert()
>>> and assume this is never NULL?
>>
>>
>> I saw you passing "radv" in the radv cache patch so thought to make it
>> optional instead of not-so-useful "radv" as gpu string.
>
>
> Far enough. I think that is a mistake on my part as we will still want the
> gpu_id in the cache entries header for identification. I need to fix it but
> I'm holding off until we get your series in.
Perhaps you should take over then, I don't have a clear vision on the
tool usage part and it now looks like I won't have time until the
weekend to work further on this.
GraÅžvydas
More information about the mesa-dev
mailing list