[Mesa-dev] [PATCH 03/88] glsl: Add initial functions to

Timothy Arceri timothy.arceri at collabora.com
Mon Sep 26 00:30:19 UTC 2016


On Sun, 2016-09-25 at 13:26 -0700, Eric Anholt wrote:
> Timothy Arceri <timothy.arceri at collabora.com> writes:
> 
> > 
> > From: Carl Worth <cworth at 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 <krh at 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.arceri at 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 <eric at 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.



More information about the mesa-dev mailing list