tailscale

Commit Graph

Author	SHA1	Message	Date
Joe Tsai	d145c594ad	util/deephash: improve cycle detection (#2470 ) The previous algorithm used a map of all visited pointers. The strength of this approach is that it quickly prunes any nodes that we have ever visited before. The detriment of the approach is that pruning is heavily dependent on the order that pointers were visited. This is especially relevant for hashing a map where map entries are visited in a non-deterministic manner, which would cause the map hash to be non-deterministic (which defeats the point of a hash). This new algorithm uses a stack of all visited pointers, similar to how github.com/google/go-cmp performs cycle detection. When we visit a pointer, we push it onto the stack, and when we leave a pointer, we pop it from the stack. Before visiting a pointer, we first check whether the pointer exists anywhere in the stack. If yes, then we prune the node. The detriment of this approach is that we may hash a node more often than before since we do not prune as aggressively. The set of visited pointers up until any node is only the path of nodes up to that node and not any other pointers that may have been visited elsewhere. This provides us deterministic hashing regardless of visit order. We can now delete hashMapFallback and associated complexity, which only exists because the previous approach was non-deterministic in the presence of cycles. This fixes a failure of the old algorithm where obviously different values are treated as equal because the pruning was too aggresive. See https://github.com/tailscale/tailscale/issues/2443#issuecomment-883653534 The new algorithm is slightly slower since it prunes less aggresively: name old time/op new time/op delta Hash-8 66.1µs ± 1% 68.8µs ± 1% +4.09% (p=0.000 n=19+19) HashMapAcyclic-8 63.0µs ± 1% 62.5µs ± 1% -0.76% (p=0.000 n=18+19) TailcfgNode-8 9.79µs ± 2% 9.88µs ± 1% +0.95% (p=0.000 n=19+17) HashArray-8 643ns ± 1% 653ns ± 1% +1.64% (p=0.000 n=19+19) However, a slower but more correct algorithm seems more favorable than a faster but incorrect algorithm. Signed-off-by: Joe Tsai <joetsai@digital-static.net>	3 years ago
Joe Tsai	d666bd8533	util/deephash: disambiguate hashing of AppendTo (#2483 ) Prepend size to AppendTo output. Fixes #2443 Signed-off-by: Joe Tsai <joetsai@digital-static.net>	3 years ago
Joe Tsai	23ad028414	util/deephash: include type as part of hash for interfaces (#2476 ) A Go interface may hold any number of different concrete types. Just because two underlying values hash to the same thing does not mean the two values are identical if they have different concrete types. As such, include the type in the hash.	3 years ago
Joe Tsai	9a0c8bdd20	util/deephash: make hash type opaque The fact that Hash returns a [sha256.Size]byte leaks details about the underlying hash implementation. This could very well be any other hashing algorithm with a possible different block size. Abstract this implementation detail away by declaring an opaque type that is comparable. While we are changing the signature of UpdateHash, rename it to just Update to reduce stutter (e.g., deephash.Update). Signed-off-by: Joe Tsai <joetsai@digital-static.net>	3 years ago
Brad Fitzpatrick	ddb8726c98	util/deephash: don't reflect.Copy if element type is a defined uint8 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	df176c82f5	util/deephash: skip alloc test under race detector Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	6dc38ff25c	util/deephash: optimize hashing of byte arrays, reduce allocs in Hash name old time/op new time/op delta Hash-6 173µs ± 4% 101µs ± 3% -41.69% (p=0.000 n=10+9) HashMapAcyclic-6 101µs ± 5% 105µs ± 3% +3.52% (p=0.001 n=9+10) TailcfgNode-6 29.4µs ± 2% 16.4µs ± 3% -44.25% (p=0.000 n=8+10) name old alloc/op new alloc/op delta Hash-6 3.60kB ± 0% 1.13kB ± 0% -68.70% (p=0.000 n=10+10) HashMapAcyclic-6 2.53kB ± 0% 2.53kB ± 0% ~ (p=0.137 n=10+8) TailcfgNode-6 528B ± 0% 0B -100.00% (p=0.000 n=10+10) name old allocs/op new allocs/op delta Hash-6 84.0 ± 0% 40.0 ± 0% -52.38% (p=0.000 n=10+10) HashMapAcyclic-6 202 ± 0% 202 ± 0% ~ (all equal) TailcfgNode-6 11.0 ± 0% 0.0 -100.00% (p=0.000 n=10+10) Updates tailscale/corp#2130 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	3962744450	util/deephash: prevent infinite loop on map cycle Fixes #2340 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	aceaa70b16	util/deephash: move funcs to methods Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	e0258ffd92	util/deephash: use keyed struct literal, fix vet Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	58f2ef6085	util/deephash: add a benchmark and some benchmark data No code changes. Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	9ae3bd0939	util/deephash: export a Hash func for use by the control plane name old time/op new time/op delta Hash-6 69.4µs ± 6% 68.4µs ± 4% ~ (p=0.286 n=9+9) HashMapAcyclic-6 115µs ± 5% 115µs ± 4% ~ (p=1.000 n=10+10) name old alloc/op new alloc/op delta Hash-6 2.29kB ± 0% 1.88kB ± 0% -18.13% (p=0.000 n=10+10) HashMapAcyclic-6 2.53kB ± 0% 2.53kB ± 0% ~ (all equal) name old allocs/op new allocs/op delta Hash-6 58.0 ± 0% 54.0 ± 0% -6.90% (p=0.000 n=10+10) HashMapAcyclic-6 202 ± 0% 202 ± 0% ~ (all equal) Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>	3 years ago
Brad Fitzpatrick	700badd8f8	util/deephash: move internal/deephash to util/deephash No code changes. Just a minor package doc addition about lack of API stability.	3 years ago

13 Commits (01d4dd331d6255bd8fe4b8e9e4197253ff59efe7)