On 01/10/2017 01:45 PM, Timothy Arceri wrote:
On Tue, 2017-01-10 at 22:41 +1100, Timothy Arceri wrote:
On Tue, 2017-01-10 at 22:34 +1100, Timothy Arceri wrote:
On Tue, 2017-01-10 at 12:20 +0200, Tapani Pälli wrote:
On 01/09/2017 05:42 PM, Emil Velikov wrote:
On 9 January 2017 at 15:36, Emil Velikov <emil.l.velikov@gmail.
co
m>
wrote:
On 9 December 2016 at 05:58, Tapani Pälli <tapani.palli@intel
.c
om
wrote:
When 'evict_random_item' attempts to remove cache content
to
make more
space, it may try to remove from new cache directory it
just
created
which only has '.tmp' content at this phase and not proper
cache files,
this will cause eviction to fail and 'test_put_and_get'
subtest
assumptions about cache size and item count won't hold.
Patch fixes this issue by adding 'exception' argument for
random file
matching so that algorithm won't pick the new directory.
Hi Tapani,
I've mentioned this over at #android-ia, but adding it here
for
posterity.
The issue itself is actually quite different - in my
experiments
with
importing SHA1 implementation to mesa, I've noticed that
there
bug
lies with a buggy (and/or undefined behaviour) SHA1
implementation
given the input data.
Namely:
As one observes the output of _mesa_sha1_compute they will
notice
that
it differs across implementations. This in itself combined
with
the
(afaict) correct assumption in test_put_and_get() that the
beginning
for the SHA1blob_key_byte_zero
[Pressed enter to quickly]
...the SHA1 matches blob_key_byte_zero (as mentioned in the
comment
of
said function) makes things work ... sometimes.
OK looks like this is going to be pretty tricky to debug.
Independent
of
this test issue however I still believe my patch is
required/correct.
There is nothing currently in cache eviction code that would
prevent
it
from picking the directory created for new entry. If it picks it,
it
does not delete anything away (since no cache files inside) and
therefore does not respect the maximum size limit set for the
cache.
There are some interesting problems to be solved/improved with
regards
to eviction. The current version of shader-cache stores everything
in
a
single directory which I intend to change so that we instead store
things in directory structures like so:
./Mesa-13.0.0/i965-BDW/
./Mesa-13.0.1/i965-BDW/
./Mesa-13.0.1/radeonsi/
This will help avoid costly fallbacks that are in the current
patchset
where we fallback to a recompile at drawtime the first time we use
Mesa
13.0.1 on a shader that was cached in Mesa-13.0.0. This will also
help
3rd parties wishing to push pre-compiled shaders to a users
machine.
In this case we would always want to evict only from older Mesa
versions one question is how much we should delete at a time.
Getting back to this patch one thing I don't understand is how we
could
hit this problem in a realistic scenario. Directories names are
only
two chars long and are hexidecimals which means a max of 256
directories it should be impossible to reach the max size before
all
possible directories are created unless the cache size is extremely
small in which case we are going to have other problems.
It may just make more sense to generate all possible directories
when
the parent directory is first created.
Oh right the problem is that its empty. Still unless the cache is
very
small this seems unlikely. Did you actually come across this problem
somehow?
Oh there is a bug report. I should have looked at the original email,
those max sizes seem a bit ridiculous.
Purpose of that test is to test if eviction works (and honors the cache
size) so the reason it sets small size is to reach the max. I'm OK with
not fixing this though but then we should maybe remove that test.
Another problem I've considered is that we always delete smaller
files
than the ones we create and go over the cache size that way although
that seems unlikely also.
Hopefully we can land it soon and get some real world testing.
So in a Tl;Dr; we want to drop the ~5 different codepaths, in
favour
of a single one, used by everyone - devs, testers, and users.
Right, I agree this would be very nice. I will still check what
supporting boringssl would mean, at least from quick glance it
has
quite
different API from others (which is also unstable ..) which did
not
encourage me very much last time.
// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
