vindexes: Efficient unicode hashing

[vmg]

Description

The current implementation of Unicode vindexes is not efficient because it depends on the x/text/collate package, which allocates a linear amount of memory based on the length of the input string being collated. This can be done much more efficiently because the value we require for a vindex is always a hash, so it can be calculated using a constant amount of memory if instead of generating a weight string and hashing it, we write directly into a hasher.

Paired on this with @dbussink.

Benchmarks:

goos: linux
goarch: amd64
pkg: vitess.io/vitess/go/vt/vtgate/vindexes
cpu: 13th Gen Intel(R) Core(TM) i9-13900K
                                │   old.txt    │               new.txt               │
                                │    sec/op    │   sec/op     vs base                │
UnicodeTest/repeat=1/MD5-16       10.003µ ± 2%   7.619µ ± 0%  -23.83% (p=0.000 n=10)
UnicodeTest/repeat=1/xxHash-16     8.714µ ± 3%   6.547µ ± 1%  -24.86% (p=0.000 n=10)
UnicodeTest/repeat=4/MD5-16        39.70µ ± 2%   29.96µ ± 1%  -24.55% (p=0.000 n=10)
UnicodeTest/repeat=4/xxHash-16     34.93µ ± 2%   26.13µ ± 1%  -25.21% (p=0.000 n=10)
UnicodeTest/repeat=16/MD5-16       159.7µ ± 3%   119.8µ ± 1%  -24.97% (p=0.000 n=10)
UnicodeTest/repeat=16/xxHash-16    135.8µ ± 4%   102.7µ ± 1%  -24.32% (p=0.000 n=10)
geomean                            37.13µ        27.99µ       -24.62%

                                │     old.txt     │              new.txt               │
                                │      B/op       │    B/op     vs base                │
UnicodeTest/repeat=1/MD5-16        12906.00 ±  7%   80.00 ± 0%  -99.38% (p=0.000 n=10)
UnicodeTest/repeat=1/xxHash-16     12814.50 ±  5%   72.00 ± 0%  -99.44% (p=0.000 n=10)
UnicodeTest/repeat=4/MD5-16        49138.50 ±  5%   80.00 ± 0%  -99.84% (p=0.000 n=10)
UnicodeTest/repeat=4/xxHash-16     50670.00 ±  4%   72.00 ± 0%  -99.86% (p=0.000 n=10)
UnicodeTest/repeat=16/MD5-16      194932.50 ± 11%   80.00 ± 0%  -99.96% (p=0.000 n=10)
UnicodeTest/repeat=16/xxHash-16   195962.50 ± 11%   72.00 ± 0%  -99.96% (p=0.000 n=10)
geomean                             48.88Ki         75.89       -99.85%

                                │   old.txt    │              new.txt               │
                                │  allocs/op   │ allocs/op   vs base                │
UnicodeTest/repeat=1/MD5-16        34.000 ± 0%   2.000 ± 0%  -94.12% (p=0.000 n=10)
UnicodeTest/repeat=1/xxHash-16     34.000 ± 0%   2.000 ± 0%  -94.12% (p=0.000 n=10)
UnicodeTest/repeat=4/MD5-16       133.000 ± 0%   2.000 ± 0%  -98.50% (p=0.000 n=10)
UnicodeTest/repeat=4/xxHash-16    133.000 ± 0%   2.000 ± 0%  -98.50% (p=0.000 n=10)
UnicodeTest/repeat=16/MD5-16      529.000 ± 0%   2.000 ± 0%  -99.62% (p=0.000 n=10)
UnicodeTest/repeat=16/xxHash-16   529.000 ± 0%   2.000 ± 0%  -99.62% (p=0.000 n=10)
geomean                             133.7        2.000       -98.50%

cc @mattlord @aquarapid

Related Issue(s)

Fixes #14398

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on the CI
• Documentation was added or is not required

Deployment Notes

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[dbussink]

This can be done much more efficiently because the value we require for a vindex is always a hash, so it can be calculated using a constant amount of memory if instead of generating a weight string and hashing it, we write directly into a hasher.

The vendoring also addresses another issue. When x/text is updated, the collation tables can be fixed for a newer Unicode version but that could (very subtlety) break a vindex since it changes what input hashes to.

Here we vendor, so we essentially set it in stone and keep it always consistent for the future.

[dbussink]

So much more memory efficient!

[vitess-bot]

Hello! 👋

This Pull Request is now handled by arewefastyet. The current HEAD and future commits will be benchmarked.

You can find the performance comparison on the arewefastyet website.

[harshit-gangal]

A general question. Does the output for the current hash functions still remain the same?

[vmg]

Yes, this is 100% backwards compatible and there are golden tests that verify it. Otherwise we'd break all existing Vitess deployments by changing the shard position of existing values!