[Mesa-dev] [PATCH v2 1/5] util/disk_cache: rename mesa cache dir and introduce cache versioning

[Timothy Arceri]

On 23/08/17 21:35, Vedran Miletić wrote:
> On 08/23/2017 08:32 AM, 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.
>> ---
>>   src/compiler/glsl/tests/cache_test.c |  6 +++--
>>   src/util/disk_cache.c                | 46 ++++++++++++++++++++++++++----------
>>   src/util/disk_cache.h                |  2 ++
>>   3 files changed, 40 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/compiler/glsl/tests/cache_test.c b/src/compiler/glsl/tests/cache_test.c
>> index af1b66fb3d..3796ce6170 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/"
>> +                             CACHE_DIR_NAME);
>>   
>>      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/"
>> +                             CACHE_DIR_NAME);
>>   
>>      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..644a911e53 100644
>> --- a/src/util/disk_cache.c
>> +++ b/src/util/disk_cache.c
>> @@ -51,20 +51,34 @@
>>   
>>   /* 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)
>>   
>> +/* 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.
>> + *
>> + * - The cache version is checked internally when reading a cache entry. If we
>> + *   ever have a mismatch we are in big trouble as this means we had a cache
>> + *   collision. In case of such an event please check the skys for giant
>> + *   asteroids and that the entire Mesa team hasn't been eaten by wolves.
>> + *
>> + * - There is no strict requirement that cache versions be backwards
>> + *   compatible but effort should be taken to limit disruption where possible.
>> + */
>> +#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];
>>   
>> @@ -153,20 +167,25 @@ 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;
>>   }
>>   
>> +#define DRV_KEY_CPY(_dst, _src, _src_size) { \
>> +   memcpy(_dst, _src, _src_size);            \
>> +   _dst += _src_size;                        \
>> +} while (0)
>> +
>>   struct disk_cache *
>>   disk_cache_create(const char *gpu_name, const char *timestamp,
>>                     uint64_t driver_flags)
>>   {
>>      void *local;
>>      struct disk_cache *cache = NULL;
>>      char *path, *max_size_str;
>>      uint64_t max_size;
>>      int fd = -1;
>>      struct stat sb;
>> @@ -181,41 +200,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
>>       */
> 
> Nitpick: since it's being changed, can it be
> $XDG_CACHE_HOME/mesa-shader-cache instead of
> $XDG_CACHE_HOME/mesa_shader_cache? I have seen both dashes and
> underscores in the wild, but dashes tend to be more frequent.

I'm sure it could be changed but we already have software written based 
around this name so I'd rather just leave as is.

> 
> Regards,
> Vedran