On Tue, Mar 14, 2017 at 12:20 AM, Timothy Arceri <tarc...@itsqueeze.com> wrote:
>
>
> On 13/03/17 22:53, Grazvydas Ignotas wrote:
>>
>> On Mon, Mar 13, 2017 at 3:01 AM, Timothy Arceri <tarc...@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.
I agree an interface just for the test might be too much. Could maybe
use a polling loop checking if the entry landed every 100ms or so and
giving up after some retries? Could also just stick a 300ms (or
similar) sleep but I fear it may cause spurious test failures on busy
systems. I think wine has that problem with their tests as they rely
on things happening after fixed sleep periods a lot.
GraÅžvydas
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
