[Mesa-dev] [PATCH 4/4] util/disk_cache: use a thread queue to write to shader cache

[Timothy Arceri]

On 13/03/17 22:53, Grazvydas Ignotas wrote:
> On Mon, Mar 13, 2017 at 3:01 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> This should help reduce any overhead added by the shader cache
>> when programs are not found in the cache.
>>
>> To avoid creating any special function just for the sake of the
>> tests we add a one second delay whenever we call dick_cache_put()
>> to give it time to finish.
>> ---
>>  src/compiler/glsl/tests/cache_test.c | 49 +++++++++++++++++++++++++++++++-----
>>  src/util/disk_cache.c                | 41 ++++++++++++++++++++----------
>>  2 files changed, 71 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/compiler/glsl/tests/cache_test.c b/src/compiler/glsl/tests/cache_test.c
>> index 6451e65..f8259e0 100644
>> --- a/src/compiler/glsl/tests/cache_test.c
>> +++ b/src/compiler/glsl/tests/cache_test.c
>> @@ -25,20 +25,21 @@
>>
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <stdbool.h>
>>  #include <string.h>
>>  #include <ftw.h>
>>  #include <errno.h>
>>  #include <stdarg.h>
>>  #include <inttypes.h>
>>  #include <limits.h>
>> +#include <time.h>
>>  #include <unistd.h>
>>
>>  #include "util/mesa-sha1.h"
>>  #include "util/disk_cache.h"
>>
>>  bool error = false;
>>
>>  #ifdef ENABLE_SHADER_CACHE
>>
>>  static void
>> @@ -224,20 +225,27 @@ does_cache_contain(struct disk_cache *cache, cache_key key)
>>
>>     if (result) {
>>        free(result);
>>        return true;
>>     }
>>
>>     return false;
>>  }
>>
>>  static void
>> +wait_one_second()
>> +{
>> +    unsigned wait_time = time(0) + 1;
>> +    while (time(0) < wait_time);
>> +}
>
> If you enter this function at an unfortunate time (just as the second
> is about to change) the function will not wait at all, so this will
> cause spurious test failures. If you just sleep() it will waste
> everyone's time on make check.
>
> It looks like flushing the queue in disk_cache_get() should solve
> this, it might be needed anyway because the app may request something
> while the thread is still writing it out?
>

I agree the wait is not ideal but it's more ideal that adding code just 
to pass tests. It's very unlikely an app will try to relink the same 
shader combination while we are still writing out and if it does I'm 
happy for this to trigger a relink, rather than adding more code which 
is unlikely to ever be used. If it's stuck waiting on I/O it's likely 
better to just recompile anyway.

I guess the only options other than the wait are a flush helper that is 
only used by the test or expose the cache struct and access the queue 
directly in the test. Not really sure which of those is better.