[Mesa-dev] [PATCH 1/4] util/disk_cache: rename mesa cache dir and introduce cache versioning
Nicolai Hähnle
nhaehnle at gmail.com
Fri Aug 18 13:05:37 UTC 2017
On 18.08.2017 13:45, Timothy Arceri wrote:
>
>
> On 18/08/17 21:02, Nicolai Hähnle wrote:
>> On 15.08.2017 01:26, Timothy Arceri wrote:
>>> Steam is already analysing cache items, unfortunatly we did not
>>> introduce a versioning mechanism for identifying structural changes
>>> to cache entries earlier so the only way to do so is to rename the
>>> cache directory.
>>>
>>> Since we are renaming it we take the opportunity to give the directory
>>> a more meaningful name.
>>>
>>> Adding a version field to the header of cache entries will help us to
>>> avoid having to rename the directory in future. Please note this is
>>> versioning for the internal structure of the entries as defined in
>>> disk_cache.{c,h} as opposed to the structure of the data provided to
>>> the disk cache by the GLSL compiler and the various driver backends.
>>
>> If the version is intended to reflect the disk cache-internal
>> structure of files, shouldn't you check the version when loading a file?
>
> Not really we have been relying on cache collisions being improbable up
> to this point. We can check but it doesn't add a great deal of value,
> the version is also unlikely to change very often.
Okay, so the version is basically intended as a version of the header.
That's fine.
Still, consider what happens in a theoretical update where the header
format is changed. Then loading files of the old version will likely
fail mysteriously either during inflating the payload or during the CRC
check, because Mesa will calculate the header size incorrectly. That's
not ideal.
BTW, it may be useful to put the total header size in the file as well,
so we can just skip it quickly at load, since Mesa doesn't use it. (And
probably won't use it -- checking those GLSL source hashes shouldn't be
necessary.)
>> Also, what about the index file? Shouldn't it get the version as well?
>
> The index can be rebuilt from the from the cache entries once this
> series lands. I don't think we need to bother with it.
I suppose a corrupt index doesn't really hurt (other than perhaps
forcing a delayed shader compile), so I guess it's fine.
Cheers,
Nicolai
>
>>
>> Cheers,
>> Nicolai
>>
>>
>>> ---
>>> src/compiler/glsl/tests/cache_test.c | 6 ++++--
>>> src/util/disk_cache.c | 37
>>> +++++++++++++++++++++++++-----------
>>> 2 files changed, 30 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/tests/cache_test.c
>>> b/src/compiler/glsl/tests/cache_test.c
>>> index af1b66fb3d..aa779e3985 100644
>>> --- a/src/compiler/glsl/tests/cache_test.c
>>> +++ b/src/compiler/glsl/tests/cache_test.c
>>> @@ -178,38 +178,40 @@ test_disk_cache_create(void)
>>> /* Test with XDG_CACHE_HOME set */
>>> setenv("XDG_CACHE_HOME", CACHE_TEST_TMP "/xdg-cache-home", 1);
>>> cache = disk_cache_create("test", "make_check", 0);
>>> expect_null(cache, "disk_cache_create with XDG_CACHE_HOME set with"
>>> "a non-existing parent directory");
>>> mkdir(CACHE_TEST_TMP, 0755);
>>> cache = disk_cache_create("test", "make_check", 0);
>>> expect_non_null(cache, "disk_cache_create with XDG_CACHE_HOME
>>> set");
>>> - check_directories_created(CACHE_TEST_TMP "/xdg-cache-home/mesa");
>>> + check_directories_created(CACHE_TEST_TMP
>>> + "/xdg-cache-home/mesa_shader_cache");
>>> 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", 0);
>>> expect_null(cache, "disk_cache_create with MESA_GLSL_CACHE_DIR
>>> set with"
>>> "a non-existing parent directory");
>>> mkdir(CACHE_TEST_TMP, 0755);
>>> cache = disk_cache_create("test", "make_check", 0);
>>> expect_non_null(cache, "disk_cache_create with
>>> MESA_GLSL_CACHE_DIR set");
>>> - check_directories_created(CACHE_TEST_TMP
>>> "/mesa-glsl-cache-dir/mesa");
>>> + check_directories_created(CACHE_TEST_TMP
>>> + "/mesa-glsl-cache-dir/mesa_shader_cache");
>>> disk_cache_destroy(cache);
>>> }
>>> static bool
>>> does_cache_contain(struct disk_cache *cache, const 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 b2229874e0..283856b15c 100644
>>> --- a/src/util/disk_cache.c
>>> +++ b/src/util/disk_cache.c
>>> @@ -51,20 +51,28 @@
>>> /* Number of bits to mask off from a cache key to get an index. */
>>> #define CACHE_INDEX_KEY_BITS 16
>>> /* Mask for computing an index from a key. */
>>> #define CACHE_INDEX_KEY_MASK ((1 << CACHE_INDEX_KEY_BITS) - 1)
>>> /* The number of keys that can be stored in the index. */
>>> #define CACHE_INDEX_MAX_KEYS (1 << CACHE_INDEX_KEY_BITS)
>>> +#define CACHE_DIR_NAME "mesa_shader_cache"
>>> +
>>> +/* The cache version should be bumped whenever a change is made to the
>>> + * structure of cache entries or the index. This will give any 3rd
>>> party
>>> + * applications reading the cache entries a chance to adjust to the
>>> changes.
>>> + */
>>> +#define CACHE_VERSION 1
>>> +
>>> struct disk_cache {
>>> /* The path to the cache directory. */
>>> char *path;
>>> /* Thread queue for compressing and writing cache entries to
>>> disk */
>>> struct util_queue cache_queue;
>>> /* Seed for rand, which is used to pick a random directory */
>>> uint64_t seed_xorshift128plus[2];
>>> @@ -181,41 +189,41 @@ disk_cache_create(const char *gpu_name, const
>>> char *timestamp,
>>> if (local == NULL)
>>> goto fail;
>>> /* At user request, disable shader cache entirely. */
>>> if (getenv("MESA_GLSL_CACHE_DISABLE"))
>>> goto fail;
>>> /* Determine path for cache based on the first defined name as
>>> follows:
>>> *
>>> * $MESA_GLSL_CACHE_DIR
>>> - * $XDG_CACHE_HOME/mesa
>>> - * <pwd.pw_dir>/.cache/mesa
>>> + * $XDG_CACHE_HOME/mesa_shader_cache
>>> + * <pwd.pw_dir>/.cache/mesa_shader_cache
>>> */
>>> path = getenv("MESA_GLSL_CACHE_DIR");
>>> if (path) {
>>> if (mkdir_if_needed(path) == -1)
>>> goto fail;
>>> - path = concatenate_and_mkdir(local, path, "mesa");
>>> + path = concatenate_and_mkdir(local, path, CACHE_DIR_NAME);
>>> 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 = concatenate_and_mkdir(local, xdg_cache_home, "mesa");
>>> + path = concatenate_and_mkdir(local, xdg_cache_home,
>>> CACHE_DIR_NAME);
>>> if (path == NULL)
>>> goto fail;
>>> }
>>> }
>>> if (path == NULL) {
>>> char *buf;
>>> size_t buf_size;
>>> struct passwd pwd, *result;
>>> @@ -237,21 +245,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 = concatenate_and_mkdir(local, path, "mesa");
>>> + path = concatenate_and_mkdir(local, path, CACHE_DIR_NAME);
>>> 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)
>>> @@ -337,46 +345,53 @@ disk_cache_create(const char *gpu_name, const
>>> char *timestamp,
>>> cache->max_size = max_size;
>>> /* 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, 0);
>>> + uint8_t cache_version = CACHE_VERSION;
>>> + size_t cv_size = sizeof(cache_version);
>>> + cache->driver_keys_blob_size = cv_size;
>>> +
>>> /* 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 += 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.
>>> */
>>> uint8_t ptr_size = sizeof(void *);
>>> size_t ptr_size_size = sizeof(ptr_size);
>>> cache->driver_keys_blob_size += ptr_size_size;
>>> size_t driver_flags_size = sizeof(driver_flags);
>>> cache->driver_keys_blob_size += driver_flags_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,
>>> - ptr_size_size);
>>> - memcpy(cache->driver_keys_blob + ts_size + gpu_name_size +
>>> ptr_size_size,
>>> + memcpy(cache->driver_keys_blob, &cache_version, cv_size);
>>> + memcpy(cache->driver_keys_blob + cv_size, timestamp, ts_size);
>>> + memcpy(cache->driver_keys_blob + cv_size + ts_size, gpu_name,
>>> + gpu_name_size);
>>> + memcpy(cache->driver_keys_blob + cv_size + ts_size + gpu_name_size,
>>> + &ptr_size, ptr_size_size);
>>> + memcpy(cache->driver_keys_blob + cv_size + ts_size +
>>> + gpu_name_size + ptr_size_size,
>>> &driver_flags, driver_flags_size);
>>> /* Seed our rand function */
>>> s_rand_xorshift128plus(cache->seed_xorshift128plus, true);
>>> ralloc_free(local);
>>> return cache;
>>> fail:
>>>
>>
>>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list