Commit Graph

10 Commits

Author SHA1 Message Date
Josh Bleecher Snyder
ce7a87e5e4 internal/deephash: use hash.BlockSize instead of a constant
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
135b641332 internal/deephash: add re-usable scratch space
name    old time/op    new time/op    delta
Hash-8    13.9µs ± 0%    12.5µs ± 0%  -10.10%  (p=0.008 n=5+5)

name    old alloc/op   new alloc/op   delta
Hash-8      793B ± 0%      793B ± 0%     ~     (all equal)

name    old allocs/op  new allocs/op  delta
Hash-8      14.0 ± 0%      12.0 ± 0%  -14.29%  (p=0.008 n=5+5)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
988dfcabef internal/deephash: simplify API
Reduce to just a single external endpoint.
Convert from a variadic number of interfaces to a slice there.

name    old time/op    new time/op    delta
Hash-8    14.4µs ± 0%    14.0µs ± 1%   -3.08%  (p=0.000 n=9+9)

name    old alloc/op   new alloc/op   delta
Hash-8      873B ± 0%      793B ± 0%   -9.16%  (p=0.000 n=9+6)

name    old allocs/op  new allocs/op  delta
Hash-8      18.0 ± 0%      14.0 ± 0%  -22.22%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
b371588ce6 internal/deephash: use netaddr AppendTo methods
Slightly slower, but lots less garbage.
We will recover the speed lost in a follow-up commit.

name    old time/op    new time/op    delta
Hash-8    13.5µs ± 1%    14.3µs ± 0%   +5.84%  (p=0.000 n=10+9)

name    old alloc/op   new alloc/op   delta
Hash-8    1.46kB ± 0%    0.87kB ± 0%  -40.10%  (p=0.000 n=7+10)

name    old allocs/op  new allocs/op  delta
Hash-8      43.0 ± 0%      18.0 ± 0%  -58.14%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
09afb8e35b internal/deephash: re-use map iteration values
This requires changes to the Go toolchain.
The changes are upstream at https://golang.org/cl/320929.
They haven't been pulled into our fork yet.

No need to allocate new iteration scratch values for every map.

name              old time/op    new time/op    delta
Hash-8              13.6µs ± 0%    13.5µs ± 0%   -1.01%  (p=0.008 n=5+5)
HashMapAcyclic-8    21.2µs ± 1%    21.1µs ± 2%     ~     (p=0.310 n=5+5)

name              old alloc/op   new alloc/op   delta
Hash-8              1.58kB ± 0%    1.46kB ± 0%   -7.60%  (p=0.008 n=5+5)
HashMapAcyclic-8      152B ± 0%      128B ± 0%  -15.79%  (p=0.008 n=5+5)

name              old allocs/op  new allocs/op  delta
Hash-8                49.0 ± 0%      43.0 ± 0%  -12.24%  (p=0.008 n=5+5)
HashMapAcyclic-8      4.00 ± 0%      2.00 ± 0%  -50.00%  (p=0.008 n=5+5)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
a2d7a2aeb1 internal/deephash: use MapIter.Set{Key,Value}
To get the benefit of this optimization requires help from the Go toolchain.
The changes are upstream at https://golang.org/cl/320929,
and have been pulled into the Tailscale fork at
728ecc58fd.
It also requires building with the build tag tailscale_go.

name              old time/op    new time/op    delta
Hash-8              14.0µs ± 0%    13.6µs ± 0%   -2.88%  (p=0.008 n=5+5)
HashMapAcyclic-8    24.3µs ± 1%    21.2µs ± 1%  -12.47%  (p=0.008 n=5+5)

name              old alloc/op   new alloc/op   delta
Hash-8              2.16kB ± 0%    1.58kB ± 0%  -27.01%  (p=0.008 n=5+5)
HashMapAcyclic-8    2.53kB ± 0%    0.15kB ± 0%  -93.99%  (p=0.008 n=5+5)

name              old allocs/op  new allocs/op  delta
Hash-8                77.0 ± 0%      49.0 ± 0%  -36.36%  (p=0.008 n=5+5)
HashMapAcyclic-8       202 ± 0%         4 ± 0%  -98.02%  (p=0.008 n=5+5)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>

setkey
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
020e904f4e internal/deephash: add special handling for netaddr.IPPort
The acyclic map code interacts badly with netaddr.IPs.
One of the netaddr.IP fields is an *intern.Value,
and we use a few sentinel values.
Those sentinel values make many of the netaddr data structures appear cyclic.

One option would be to replace the cycle-detection code with
a Floyd-Warshall style algorithm. The downside is that this will take
longer to detect cycles, particularly if the cycle is long.

This problem is exacerbated by the fact that the acyclic cycle detection
code shares a single visited map for the entire data structure,
not just the subsection of the data structure localized to the map.
Unfortunately, the extra allocations and work (and code) to use per-map
visited maps make this option not viable.

Instead, continue to special-case netaddr data types.

name              old time/op    new time/op    delta
Hash-8              22.4µs ± 0%    14.0µs ± 0%  -37.59%  (p=0.008 n=5+5)
HashMapAcyclic-8    23.8µs ± 0%    24.3µs ± 1%   +1.75%  (p=0.008 n=5+5)

name              old alloc/op   new alloc/op   delta
Hash-8              2.49kB ± 0%    2.16kB ± 0%     ~     (p=0.079 n=4+5)
HashMapAcyclic-8    2.53kB ± 0%    2.53kB ± 0%     ~     (all equal)

name              old allocs/op  new allocs/op  delta
Hash-8                86.0 ± 0%      77.0 ± 0%  -10.47%  (p=0.008 n=5+5)
HashMapAcyclic-8       202 ± 0%       202 ± 0%     ~     (all equal)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Josh Bleecher Snyder
bbb79f2d6a internal/deephash: fix accidental naked return
name              old time/op    new time/op    delta
Hash-8              23.0µs ± 1%    22.4µs ± 0%   -2.43%  (p=0.008 n=5+5)
HashMapAcyclic-8    24.0µs ± 0%    23.8µs ± 0%   -0.56%  (p=0.008 n=5+5)

name              old alloc/op   new alloc/op   delta
Hash-8              2.92kB ± 0%    2.49kB ± 0%  -14.80%  (p=0.000 n=5+4)
HashMapAcyclic-8    2.53kB ± 0%    2.53kB ± 0%     ~     (all equal)

name              old allocs/op  new allocs/op  delta
Hash-8                93.0 ± 0%      86.0 ± 0%   -7.53%  (p=0.008 n=5+5)
HashMapAcyclic-8       202 ± 0%       202 ± 0%     ~     (all equal)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
2021-05-24 13:51:23 -07:00
Brad Fitzpatrick
79b7fa9ac3 internal/deephash: hash maps without sorting in the acyclic common case
Hash and xor each entry instead, then write final xor'ed result.

name    old time/op    new time/op    delta
Hash-4    33.6µs ± 4%    34.6µs ± 3%  +3.03%  (p=0.013 n=10+9)

name    old alloc/op   new alloc/op   delta
Hash-4    1.86kB ± 0%    1.77kB ± 0%  -5.10%  (p=0.000 n=10+9)

name    old allocs/op  new allocs/op  delta
Hash-4      51.0 ± 0%      49.0 ± 0%  -3.92%  (p=0.000 n=10+10)

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-05-24 13:51:23 -07:00
Brad Fitzpatrick
36a26e6a71 internal/deephash: rename from deepprint
Yes, it printed, but that was an implementation detail for hashing.

And coming optimization will make it print even less.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2021-05-11 12:11:16 -07:00