[Mesa-dev] [PATCH 2/4] util/disk_cache: add struct cache_item_metadata

[Pierre-Loup A. Griffais]

On 08/18/2017 04:02 AM, Nicolai Hähnle wrote:
> On 15.08.2017 01:26, Timothy Arceri wrote:
>> This will be used to store more information about the cache item
>> in it's header. This information is intended for 3rd party and
>> cache analysis use but can also be used for detecting the unlikely
>> scenario of cache collisions.
>> ---
>>   src/util/disk_cache.h | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/src/util/disk_cache.h b/src/util/disk_cache.h
>> index 9aade16a9e..d6d7609ffb 100644
>> --- a/src/util/disk_cache.h
>> +++ b/src/util/disk_cache.h
>> @@ -34,20 +34,42 @@
>>   #ifdef __cplusplus
>>   extern "C" {
>>   #endif
>>   /* Size of cache keys in bytes. */
>>   #define CACHE_KEY_SIZE 20
>>   typedef uint8_t cache_key[CACHE_KEY_SIZE];
>> +/* WARNING: 3rd party applications might be reading the cache item 
>> metadata.
>> + * Do not change these values without making the change widely known.
>> + * Please contact Valve developers and make them aware of this change.
>> + */
>> +#define CACHE_ITEM_TYPE_UNKNOWN  0x0
>> +#define CACHE_ITEM_TYPE_GLSL     0x1
>> +
>> +typedef uint32_t cache_item_metadata_type;
> 
> Please don't do typedefs like that, they just obfuscate. If you don't 
> want to just use uint32_t, I'd suggest to use an enum.
> 
> At a higher level, what is this actually good for? So I get Valve wants 
> to parse something in the shader cache, but why / what for?

Steam looks at local entries and delete leftover ones should you upgrade 
Mesa or your GPU. It'll also collect individual cache entries and send 
them to other users with a matching GPU or Mesa version if deemed safe, 
meaning they won't have to build these shaders locally.

> 
> Cheers,
> Nicolai
> 
> 
>> +
>> +struct cache_item_metadata {
>> +   /**
>> +    * The cache item type. This could be used to identify a GLSL 
>> cache item,
>> +    * a certain type of IR (tgsi, nir, etc), or signal that it is the 
>> final
>> +    * binary form of the shader.
>> +    */
>> +   cache_item_metadata_type type;
>> +
>> +   /** GLSL cache item metadata */
>> +   cache_key *keys;   /* sha1 list of shaders that make up the cache 
>> item */
>> +   uint32_t num_keys;
>> +};
>> +
>>   struct disk_cache;
>>   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;
>>
> 
>