We were folding together a string of 4 byte values as 8 byte values,
causing us to CRC in quite a few zeros. This appears to really poison
the algorithm, resulting in frequent hash collisions.
Change-Id: I1e363088d821d2fc0bbc392b78d1e24690fdc70a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432938
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This reverts commit 3c161467f0.
Reason for revert: need to keep iOS simulator (Build-Mac-Clang-x64-Release-iOS) in mind
Original change's description:
> restore murmur3 for older iOS devices
>
> For reference, the relative costs are roughly,
> - our hash with CRC32c instructions 1x
> - Murmur3 11x
> - our hash with CRC32c fallback 23x
>
> So this should be a ~2x speedup for those
> older iOS devices not using an arm64e slice.
>
> Bug: skia:11001
> Cq-Include-Trybots: luci.skia.skia.primary:Test-iOS-Clang-iPadPro-GPU-PowerVRGT7800-arm64-Debug-All,Test-iOS-Clang-iPhone11-GPU-AppleA13-arm64-Debug-All,Test-iOS-Clang-iPhone6-GPU-PowerVRGX6450-arm64-Debug-All,Test-iOS-Clang-iPhone7-GPU-PowerVRGT7600-arm64-Debug-All,Test-iOS-Clang-iPhone8-GPU-AppleA11-arm64-Debug-All
> Change-Id: Ib56195ddc0c522380d263d56e767331d9f635728
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340178
> Commit-Queue: Mike Klein <mtklein@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=mtklein@google.com,brianosman@google.com
Change-Id: I0e3b23e63c33910e482031d7475feb624bd6e1f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11001
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340396
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
For reference, the relative costs are roughly,
- our hash with CRC32c instructions 1x
- Murmur3 11x
- our hash with CRC32c fallback 23x
So this should be a ~2x speedup for those
older iOS devices not using an arm64e slice.
Bug: skia:11001
Cq-Include-Trybots: luci.skia.skia.primary:Test-iOS-Clang-iPadPro-GPU-PowerVRGT7800-arm64-Debug-All,Test-iOS-Clang-iPhone11-GPU-AppleA13-arm64-Debug-All,Test-iOS-Clang-iPhone6-GPU-PowerVRGX6450-arm64-Debug-All,Test-iOS-Clang-iPhone7-GPU-PowerVRGT7600-arm64-Debug-All,Test-iOS-Clang-iPhone8-GPU-AppleA11-arm64-Debug-All
Change-Id: Ib56195ddc0c522380d263d56e767331d9f635728
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340178
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This new version always delivers the same results,
and I think can be simplified like this without
spoiling any of the bulk speed.
Change-Id: I20e42e58418e658278bb5db9472c39722b33160a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338339
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Current strategy: everything from the top
Things to look at first are the manual changes:
- added tools/rewrite_includes.py
- removed -Idirectives from BUILD.gn
- various compile.sh simplifications
- tweak tools/embed_resources.py
- update gn/find_headers.py to write paths from the top
- update gn/gn_to_bp.py SkUserConfig.h layout
so that #include "include/config/SkUserConfig.h" always
gets the header we want.
No-Presubmit: true
Change-Id: I73a4b181654e0e38d229bc456c0d0854bae3363e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209706
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
We noticed a hash collision scenario we'd like to avoid, and it looks
pretty easy. The new test failed on my desktop at both asserts,
depending on whether I build 32- or 64-bit, and also on my
CRC-supporting phone. The Murmur3 path should be fine.
Change-Id: I005de38d68f9b0461536c29e48cf7e5be5caac3e
Reviewed-on: https://skia-review.googlesource.com/c/159621
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 1a462bd0fe.
Reason for revert: Broke android. Need to run android bot for these.
Original change's description:
> IWYU for some test files starting with 'C'.
>
> Change-Id: I9a9596f7a941cdd8f01e055965c70a4b24438499
> Reviewed-on: https://skia-review.googlesource.com/113746
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
TBR=bungeman@google.com,herb@google.com
Change-Id: Ic63c2fcc7457e442e9b29a9ccd429927e24e3b77
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/113841
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
About 9x faster than Murmur3 for long inputs.
Most of this is a mechanical change from SkChecksum::Murmur3(...) to SkOpts::hash(...).
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2208903002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot;master.client.skia.compile:Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot,Build-Mac-Clang-x86_64-Release-CMake-Trybot
Review-Url: https://codereview.chromium.org/2208903002
SkChecksum::Compute is a very, very poorly distributed hash function.
This replaces all remaining uses with Murmur3.
The only interesting stuff is in src/gpu.
BUG=skia:
Review URL: https://codereview.chromium.org/1436973003
Passing &SkGoodHash to SkTHashMap and SkTHashSet doesn't guarantee that it's actually instantiated. Using a functor does.
BUG=skia:
Review URL: https://codereview.chromium.org/1405053002
- By default, use new SkGoodHash to hash keys, which is:
* for 4 byte values, use SkChecksum::Mix,
* for SkStrings, use SkChecksum::Murmur3 on the data,
* for other structs, shallow hash the struct with Murmur3.
- Expand SkChecksum::Murmur3 to support non-4-byte-aligned data.
- Add const foreach() methods.
- Have foreach() take a functor, which allows lambdas.
BUG=skia:
Review URL: https://codereview.chromium.org/1021033002
