The API and implementation are very much simplified.
You may not want to bother reading the diff.
As is our trend, SkOnce now uses <atomic> directly.
Member initialization means we don't need SK_DECLARE_STATIC_ONCE.
SkSpinlock already works this same way.
All uses of the old API taking an external bool* and Lock* were pessimal,
so I have not carried this sort of API forward. It's simpler, faster,
and more space-efficient to always use this single SkOnce class interface.
SkOnce weighs 2 bytes: a done bool and an SkSpinlock, also a bool internally.
This API refactoring opens up the opportunity to fuse those into a single
three-state byte if we'd like.
No public API changes.
TBR=reed@google.com
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1894893002
Review URL: https://codereview.chromium.org/1894893002
SkTemplates.h contains a number of Skia specific utilities which are
not designed for external use. In addition to reducing the external
support burden, this will allow Skia to freely refactor this file.
Review URL: https://codereview.chromium.org/1272293004
Paths are cached as tessellated triangle meshes in vertex buffers on the GPU. Stroked paths are not (yet) cached.
Paths containing no curved segments (linear paths) are reused at all scales. Paths containing curved segments are reused within a scale tolerance threshold.
In order to invalidate the cache when an SkPath is changed or deleted,
this required implementing genID change notification in SkPath. This is
modelled almost exactly on SkPixelRef::GenIDChangeListener.
However, It does not currently implement the check for unique genIDs,
so notifiers will fire when the first instance of an SkPathRef
using a given genID is destroyed.
Another caveat is that you cannot successfully add a change notifier
to an empty path, since it uses the "canonical" empty path which is
never modified or destroyed. For this reason, we prevent adding
listeners to it.
BUG=skia:4121,skia:4122, 497403
DOCS_PREVIEW= https://skia.org/?cl=1114353004
Committed: https://skia.googlesource.com/skia/+/468dfa72eb6694145487be17876804dfca3b7adb
Review URL: https://codereview.chromium.org/1114353004
Paths are cached as tessellated triangle meshes in vertex buffers on the GPU. Stroked paths are not (yet) cached.
Paths containing no curved segments (linear paths) are reused at all scales. Paths containing curved segments are reused within a scale tolerance threshold.
In order to invalidate the cache when an SkPath is changed or deleted,
this required implementing genID change notification in SkPath. This is
modelled almost exactly on SkPixelRef::GenIDChangeListener.
However, It does not currently implement the check for unique genIDs,
so notifiers will fire when the first instance of an SkPathRef
using a given genID is destroyed.
Another caveat is that you cannot successfully add a change notifier
to an empty path, since it uses the "canonical" empty path which is
never modified or destroyed. For this reason, we prevent adding
listeners to it.
BUG=skia:4121,skia:4122, 497403
DOCS_PREVIEW= https://skia.org/?cl=1114353004
Review URL: https://codereview.chromium.org/1114353004
Make GrResourceCache performance less sensitive to key length change.
The memcmp in GrResourceKey is called when SkTDynamicHash jumps the
slots to find the hash by a index. Avoid most of the memcmps by
comparing the hash first.
This is important because small changes in key data length can cause
big performance regressions. The theory is that key length change causes
different hash values. These hash values might trigger memcmps that
originally weren't there, causing the regression.
Adds few specialized benches to grresourcecache_add to test different
key lengths. The tests are run only on release, because on debug the
SkTDynamicHash validation takes too long, and adding many such delays
to development test runs would be unproductive. On release the tests
are quite fast.
Effect of this patch to the added tests on amd64:
grresourcecache_find_10 738us -> 768us 1.04x
grresourcecache_find_2 472us -> 476us 1.01x
grresourcecache_find_25 841us -> 845us 1x
grresourcecache_find_4 565us -> 531us 0.94x
grresourcecache_find_54 1.18ms -> 1.1ms 0.93x
grresourcecache_find_5 834us -> 749us 0.9x
grresourcecache_find_3 620us -> 542us 0.87x
grresourcecache_add_25 2.74ms -> 2.24ms 0.82x
grresourcecache_add_56 3.23ms -> 2.56ms 0.79x
grresourcecache_add_54 3.34ms -> 2.62ms 0.78x
grresourcecache_add_5 2.68ms -> 2.1ms 0.78x
grresourcecache_add_10 2.7ms -> 2.11ms 0.78x
grresourcecache_add_2 1.85ms -> 1.41ms 0.76x
grresourcecache_add 1.84ms -> 1.4ms 0.76x
grresourcecache_add_4 1.99ms -> 1.49ms 0.75x
grresourcecache_add_3 2.11ms -> 1.55ms 0.73x
grresourcecache_add_55 39ms -> 13.9ms 0.36x
grresourcecache_find_55 23.2ms -> 6.21ms 0.27x
On arm64 the results are similar.
On arm_v7_neon, the results lack the discontinuity at 55:
grresourcecache_add 4.06ms -> 4.26ms 1.05x
grresourcecache_add_2 4.05ms -> 4.23ms 1.05x
grresourcecache_find 1.28ms -> 1.3ms 1.02x
grresourcecache_find_56 3.35ms -> 3.32ms 0.99x
grresourcecache_find_2 1.31ms -> 1.29ms 0.99x
grresourcecache_find_54 3.28ms -> 3.24ms 0.99x
grresourcecache_add_5 6.38ms -> 6.26ms 0.98x
grresourcecache_add_55 8.44ms -> 8.24ms 0.98x
grresourcecache_add_25 7.03ms -> 6.86ms 0.98x
grresourcecache_find_25 2.7ms -> 2.59ms 0.96x
grresourcecache_find_4 1.45ms -> 1.38ms 0.95x
grresourcecache_find_10 2.52ms -> 2.39ms 0.95x
grresourcecache_find_55 3.54ms -> 3.33ms 0.94x
grresourcecache_find_5 2.5ms -> 2.32ms 0.93x
grresourcecache_find_3 1.57ms -> 1.43ms 0.91x
The extremely slow case, 55, is postulated to be due to the index jump
collisions running the memcmp. This is not visible on arm_v7_neon probably due
to hash function producing different results for 32 bit architectures.
This change is needed for extending path cache key in Gr
NV_path_rendering codepath. Extending is needed in order to add dashed
paths to the path cache.
Review URL: https://codereview.chromium.org/1132723003
Reason for revert:
Our Valgrind bot just spewed out a weird error. I don't know if it's related, but it looks at least like one of the stacks was in the right area, so I'm going to revert this precautionarily. Sorry if this is a false positive.
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu12-ShuttleA-GTX550Ti-x86_64-Release-Valgrind/builds/266/steps/dm/logs/stdio
Original issue's description:
> Make GrScratchKey memory buffer correct size on copy
>
> Scratch key memory buffer of a copy of a key was too big. The (new) copy
> was N times uint32_t bytes instead of N bytes.
>
> Adds few tests to resource cache. These tests would not catch the too
> big buffer. This is just a precaution for too small buffers. The main
> idea of the test change is that the scratch key should contain some
> information, so that lookup with a scratch key can also return no
> match. Otherwise testing of scratch lookup result is not indicative of
> correct code (eg. no-information scratch key will always match).
>
> Committed: https://skia.googlesource.com/skia/+/711ef4831363fb8cbdf061dc2c36c65b13c0ccf2TBR=bsalomon@google.com,kkinnunen@nvidia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/864833003
Scratch key memory buffer of a copy of a key was too big. The (new) copy
was N times uint32_t bytes instead of N bytes.
Adds few tests to resource cache. These tests would not catch the too
big buffer. This is just a precaution for too small buffers. The main
idea of the test change is that the scratch key should contain some
information, so that lookup with a scratch key can also return no
match. Otherwise testing of scratch lookup result is not indicative of
correct code (eg. no-information scratch key will always match).
Review URL: https://codereview.chromium.org/860333002
