On Sun, 2016-09-25 at 13:26 -0700, Eric Anholt wrote:
> Timothy Arceri <timothy.arc...@collabora.com> writes:
>
> >
> > From: Carl Worth <cwo...@cworth.org>
> >
> > This code provides for an on-disk cache of objects. Objects are
> > stored
> > and retrieved via names that are arbitrary 20-byte sequences,
> > (intended to be SHA-1 hashes of something identifying for the
> > content). The directory used for the cache can be specified by
> > means
> > of environment variables in the following priority order:
> >
> > $MESA_GLSL_CACHE_DIR
> > $XDG_CACHE_HOME/mesa
> > <user-home-directory>/.cache/mesa
> >
> > By default the cache will be limited to a maximum size of 1GB. The
> > environment variable:
> >
> > $MESA_GLSL_CACHE_MAX_SIZE
> >
> > can be set (at the time of GL context creation) to choose some
> > other
> > size. This variable is a number that can optionally be followed by
> > 'K', 'M', or 'G' to select a size in kilobytes, megabytes, or
> > gigabytes. By default, an unadorned value will be interpreted as
> > gigabytes.
> >
> > The cache will be entirely disabled at runtime if the variable
> > MESA_GLSL_CACHE_DISABLE is set at the time of GL context creation.
> >
> > Many thanks to Kristian Høgsberg <k...@bitplanet.net> for the
> > initial
> > implementation of code that led to this patch. In particular, the
> > idea
> > of using an mmapped file, (indexed by a portion of the SHA-1), for
> > the
> > efficent implementation of cache_has_key was entirely his
> > idea. Kristian also provided some very helpful advice in
> > discussions
> > regarding various race conditions to be avoided in this code.
>
> General style note: A bunch of calls in this commit put a space
> between
> function name and '(' and that should probably be fixed up for
> consistency with Mesa.
>
> >
> > Signed-off-by: Timothy Arceri <timothy.arc...@collabora.com>
> > ---
> > configure.ac | 3 +
> > docs/envvars.html | 11 +
> > src/compiler/Makefile.glsl.am | 10 +
> > src/compiler/Makefile.sources | 4 +
> > src/compiler/glsl/cache.c | 709
> > +++++++++++++++++++++++++++++++++++
> > src/compiler/glsl/cache.h | 172 +++++++++
> > src/compiler/glsl/tests/.gitignore | 1 +
> > src/compiler/glsl/tests/cache_test.c | 416 ++++++++++++++++++++
> > 8 files changed, 1326 insertions(+)
> > create mode 100644 src/compiler/glsl/cache.c
> > create mode 100644 src/compiler/glsl/cache.h
> > create mode 100644 src/compiler/glsl/tests/cache_test.c
> >
>
> >
> > diff --git a/docs/envvars.html b/docs/envvars.html
> > index cf57ca5..2375145 100644
> > --- a/docs/envvars.html
> > +++ b/docs/envvars.html
> > @@ -112,6 +112,17 @@ glGetString(GL_VERSION) for OpenGL ES.
> > glGetString(GL_SHADING_LANGUAGE_VERSION). Valid values are
> > integers, such as
> > "130". Mesa will not really implement all the features of the
> > given language version
> > if it's higher than what's normally reported. (for developers
> > only)
> > +<li>MESA_GLSL_CACHE_DISABLE - if set, disables the GLSL shader
> > cache
> > +<li>MESA_GLSL_CACHE_MAX_SIZE - if set, determines the maximum size
> > of
> > +the on-disk cache of compiled GLSL programs. Should be set to a
> > number
> > +optionally followed by 'K', 'M', or 'G' to specify a size in
> > +kilobytes, megabytes, or gigabytes. By default, gigabytes will be
> > +assumed. And if unset, a maxium size of 1GB will be used.
> > +<li>MESA_GLSL_CACHE_DIR - if set, determines the directory to be
> > used
> > +for the on-disk cache of compiled GLSL programs. If this variable
> > is
> > +not set, then the cache will be stored in $XDG_CACHE_HOME/.mesa
> > (if
>
> Looks like that's actually "$XDG_CACHE_HOME/mesa"
>
> >
> > +that variable is set), or else within .cache/mesa within the
> > user's
> > +home directory.
> > <li>MESA_GLSL - <a href="shading.html#envvars">shading language
> > compiler options</a>
> > <li>MESA_NO_MINMAX_CACHE - when set, the minmax index cache is
> > globally disabled.
> > </ul>
>
> >
> > diff --git a/src/compiler/glsl/cache.c b/src/compiler/glsl/cache.c
> > new file mode 100644
> > index 0000000..c4c4fb7
> > --- /dev/null
> > +++ b/src/compiler/glsl/cache.c
> > @@ -0,0 +1,709 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > + * and/or sell copies of the Software, and to permit persons to
> > whom the
> > + * Software is furnished to do so, subject to the following
> > conditions:
> > + *
> > + * The above copyright notice and this permission notice
> > (including the next
> > + * paragraph) shall be included in all copies or substantial
> > portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> > EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#include <ctype.h>
> > +#include <string.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <sys/file.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <sys/mman.h>
> > +#include <unistd.h>
> > +#include <fcntl.h>
> > +#include <pwd.h>
> > +#include <errno.h>
> > +#include <dirent.h>
> > +
> > +#include "util/u_atomic.h"
> > +#include "util/mesa-sha1.h"
> > +#include "util/ralloc.h"
> > +#include "main/errors.h"
> > +
> > +#include "cache.h"
> > +
> > +/* 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)
> > +
> > +struct program_cache {
> > + /* The path to the cache directory. */
> > + char *path;
> > +
> > + /* A pointer to the mmapped index file within the cache
> > directory. */
> > + uint8_t *index_mmap;
> > + size_t index_mmap_size;
> > +
> > + /* Pointer to total size of all objects in cache (within
> > index_mmap) */
> > + uint64_t *size;
> > +
> > + /* Pointer to stored keys, (within index_mmap). */
> > + uint8_t *stored_keys;
> > +
> > + /* Maximum size of all cached objects (in bytes). */
> > + uint64_t max_size;
> > +};
> > +
> > +/* Create a directory named 'path' if it does not already exist.
> > + *
> > + * Returns: 0 if path already exists as a directory or if created.
> > + * -1 in all other cases.
> > + */
> > +static int
> > +mkdir_if_needed(char *path)
> > +{
> > + struct stat sb;
> > +
> > + /* If the path exists already, then our work is done if it's a
> > + * directory, but it's an error if it is not.
> > + */
> > + if (stat(path, &sb) == 0) {
> > + if (S_ISDIR(sb.st_mode)) {
> > + return 0;
> > + } else {
> > + _mesa_warning(NULL,
> > + "Cannot use %s for shader cache (not a
> > directory)"
> > + "---disabling.\n", path);
> > + return -1;
> > + }
> > + }
> > +
> > + if (mkdir(path, 0755) == 0)
> > + return 0;
>
> This should probably be:
>
> ret = mkdir(path, 0755)
> if (ret == 0 || ret == -1 && errno == EEXIST)
> return 0;
>
> to avoid races on first creation. I guess you could leave the stat
> in
> at that point to check that the path isn't a file, but I don't care
> either way.
>
> >
> > +void
> > +cache_put(struct program_cache *cache,
> > + cache_key key,
> > + const void *data,
> > + size_t size)
> > +{
> > + int fd = -1, fd_final = -1, err, ret;
> > + size_t len;
> > + char *filename = NULL, *filename_tmp = NULL;
> > + const char *p = data;
> > +
> > + filename = get_cache_file(cache, key);
> > + if (filename == NULL)
> > + goto done;
> > +
> > + /* Write to a temporary file to allow for an atomic rename to
> > the
> > + * final destination filename, (to prevent any readers from
> > seeing
> > + * a partially written file).
> > + */
> > + filename_tmp = ralloc_asprintf(cache, "%s.tmp", filename);
> > + if (filename_tmp == NULL)
> > + goto done;
> > +
> > + fd = open(filename_tmp, O_WRONLY | O_CLOEXEC | O_CREAT, 0644);
> > +
> > + /* Make the two-character subdirectory within the cache as
> > needed. */
> > + if (fd == -1) {
> > + if (errno != ENOENT)
> > + goto done;
> > +
> > + make_cache_file_directory(cache, key);
> > +
> > + fd = open(filename_tmp, O_WRONLY | O_CLOEXEC | O_CREAT,
> > 0644);
> > + if (fd == -1)
> > + goto done;
> > + }
> > +
> > + /* With the temporary file open, we take an exclusive flock on
> > + * it. If the flock fails, then another process still has the
> > file
> > + * open with the flock held. So just let that file be
> > responsible
> > + * for writing the file.
> > + */
> > + err = flock(fd, LOCK_EX);
> > + if (err == -1)
> > + goto done;
>
> Given the comment, I think you want "| LOCK_NB" to return early if
> someone else has it locked.
>
> >
> > +static void
> > +test_put_key_and_get_key(void)
> > +{
> > + struct program_cache *cache;
> > + bool result;
> > +
> > + uint8_t key_a[20] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
> > + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19};
> > + uint8_t key_b[20] = { 20, 21, 22, 23, 24, 25, 26, 27, 28, 29,
> > + 30, 33, 32, 33, 34, 35, 36, 37, 38, 39};
> > + uint8_t key_a_collide[20] =
> > + { 0, 1, 42, 43, 44, 45, 46, 47, 48, 49,
> > + 50, 55, 52, 53, 54, 55, 56, 57, 58, 59};
> > +
> > + cache = cache_create();
> > +
> > + /* First test that cache_has_key returns false before
> > cache_put_key */
> > + result = cache_has_key(cache, key_a);
> > + expect_equal(result, 0, "cache_has_key before key added");
> > +
> > + /* Then a couple of tests of cache_put_key followed by
> > cache_has_key */
> > + cache_put_key(cache, key_a);
> > + result = cache_has_key(cache, key_a);
> > + expect_equal(result, 1, "cache_has_key after key added");
> > +
> > + cache_put_key(cache, key_b);
> > + result = cache_has_key(cache, key_b);
> > + expect_equal(result, 1, "2nd cache_has_key after key added");
> > +
> > + /* Test that a key with the same two bytes as an existing key
> > + * forces an eviction.
> > + */
> > + cache_put_key(cache, key_a_collide);
> > + result = cache_has_key(cache, key_a_collide);
> > + expect_equal(result, 1, "put_key of a colliding key lands in
> > the cache");
> > +
> > + result = cache_has_key(cache, key_a);
> > + expect_equal(result, 0, "put_key of a colliding key evicts from
> > the cache");
> > +
> > + /* And finally test that we can re-add the original key to re-
> > evict
> > + * the colliding key.
> > + */
> > + cache_put_key(cache, key_a);
> > + result = cache_has_key(cache, key_a);
> > + expect_equal(result, 1, "put_key of original key lands again");
> > +
> > + result = cache_has_key(cache, key_a_collide);
> > + expect_equal(result, 0, "put_key of oiginal key evicts the
> > colliding key");
>
> "original"
>
> I haven't yet figured out what the purpose of
> cache_put_key()/cache_has_key() are. I suppose I'll find out later
> in
> the series.
Since we cache a program rather than individual shaders we set a cache
key for each shader and opportunistically skip compiling it next time
we see the shader.
If we happen to be using the shader to create a program we haven't seen
before we end up having to fall back to compiling the shader later.
>
> With the small fixes here, this patch and patch 2 are:
>
> Reviewed-by: Eric Anholt <e...@anholt.net>
>
> This series is unfortunately way too long to review in one go, and
> I'm
> hoping we can incrementally land pieces.
That should be possible nothing should be broken by any of theses
patches and since the feature itself is disabled by default I don't see
the harm in pushing them incrementally.
Thanks for taking a look.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
