On Tue, Mar 14, 2017 at 4:06 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.
>
> V2: poll for file when waiting for thread in test
> V3: fix poll delay to really be 100ms, and simplify the wait function
> ---
> src/compiler/glsl/tests/cache_test.c | 63
> ++++++++++++++++++++++++++++++++----
> src/util/disk_cache.c | 41 +++++++++++++++--------
> 2 files changed, 85 insertions(+), 19 deletions(-)
>
> diff --git a/src/compiler/glsl/tests/cache_test.c
> b/src/compiler/glsl/tests/cache_test.c
> index 6451e65..10505d4 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,40 @@ does_cache_contain(struct disk_cache *cache, cache_key
> key)
>
> if (result) {
> free(result);
> return true;
> }
>
> return false;
> }
>
> static void
> +wait_until_file_written(struct disk_cache *cache, cache_key key)
> +{
> + struct timespec req;
> + struct timespec rem;
> +
> + /* Set 100ms delay */
> + req.tv_sec = 0;
> + req.tv_nsec = 100000000;
> +
> + unsigned retries = 0;
> + while (retries++ < 20) {
> + if (does_cache_contain(cache, key)) {
> + break;
> + }
> +
> + nanosleep(&req, &rem);
> + }
> +}
> +
> +static void
> test_put_and_get(void)
> {
> struct disk_cache *cache;
> /* If the text of this blob is changed, then blob_key_byte_zero
> * also needs to be updated.
> */
> char blob[] = "This is a blob of thirty-seven bytes";
> uint8_t blob_key[20];
> uint8_t blob_key_byte_zero = 0xca;
> char string[] = "While this string has thirty-four";
> @@ -253,35 +274,49 @@ test_put_and_get(void)
> _mesa_sha1_compute(blob, sizeof(blob), blob_key);
>
> /* Ensure that disk_cache_get returns nothing before anything is added. */
> result = disk_cache_get(cache, blob_key, &size);
> expect_null(result, "disk_cache_get with non-existent item (pointer)");
> expect_equal(size, 0, "disk_cache_get with non-existent item (size)");
>
> /* Simple test of put and get. */
> disk_cache_put(cache, blob_key, blob, sizeof(blob), NULL);
>
> + /* disk_cache_put() hands things off to a thread give it some time to
> + * finsih.
finish
> + */
> + wait_until_file_written(cache, blob_key);
> +
> result = disk_cache_get(cache, blob_key, &size);
> - expect_equal_str(blob, result, "disk_cache_get of existing item
> (pointer)");
> - expect_equal(size, sizeof(blob), "disk_cache_get of existing item
> (size)");
> + if (result) {
> + expect_equal_str(blob, result, "disk_cache_get of existing item
> (pointer)");
> + expect_equal(size, sizeof(blob), "disk_cache_get of existing item
> (size)");
>
> - free(result);
> + free(result);
> + }
>
> /* Test put and get of a second item. */
> _mesa_sha1_compute(string, sizeof(string), string_key);
> disk_cache_put(cache, string_key, string, sizeof(string), NULL);
>
> + /* disk_cache_put() hands things off to a thread give it some time to
> + * finsih.
> + */
finish (do we need to repeat the comment every time?)
> + wait_until_file_written(cache, string_key);
> +
> result = disk_cache_get(cache, string_key, &size);
> - expect_equal_str(result, string, "2nd disk_cache_get of existing item
> (pointer)");
> - expect_equal(size, sizeof(string), "2nd disk_cache_get of existing item
> (size)");
> + if (result) {
> + expect_equal_str(result, string, "2nd disk_cache_get of existing item
> (pointer)");
> + expect_equal(size, sizeof(string), "2nd disk_cache_get of existing
> item (size)");
>
> - free(result);
> + free(result);
> + }
Here (and before) you check disk_cache_get() result for NULL ,,,
>
> /* Set the cache size to 1KB and add a 1KB item to force an eviction. */
> disk_cache_destroy(cache);
>
> setenv("MESA_GLSL_CACHE_MAX_SIZE", "1K", 1);
> cache = disk_cache_create("test", "make_check");
>
> one_KB = calloc(1, 1024);
>
> /* Obviously the SHA-1 hash of 1024 zero bytes isn't particularly
> @@ -299,20 +334,25 @@ test_put_and_get(void)
> * interestingly full than that).
> *
> * For this test, we force this signature to land in the same
> * directory as the original blob first written to the cache.
> */
> _mesa_sha1_compute(one_KB, 1024, one_KB_key);
> one_KB_key[0] = blob_key_byte_zero;
>
> disk_cache_put(cache, one_KB_key, one_KB, 1024, one_KB);
>
> + /* disk_cache_put() hands things off to a thread give it some time to
> + * finish.
> + */
> + wait_until_file_written(cache, one_KB_key);
> +
> result = disk_cache_get(cache, one_KB_key, &size);
> expect_non_null(result, "3rd disk_cache_get of existing item (pointer)");
> expect_equal(size, 1024, "3rd disk_cache_get of existing item (size)");
... and here you don't. This should probably be made consistent. I
don't think the NULL checking is needed though, the whole point of the
test is to check if things work as expected.
Other than that:
Reviewed-by: Grazvydas Ignotas <nota...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
