util/deephash: use unsafe.Pointer instead of reflect.Value (#5459)

Use of reflect.Value.SetXXX panics if the provided argument was
obtained from an unexported struct field.
Instead, pass an unsafe.Pointer around and convert to a
reflect.Value when necessary (i.e., for maps and interfaces).
Converting from unsafe.Pointer to reflect.Value guarantees that
none of the read-only bits will be populated.

When running in race mode, we attach type information to the pointer
so that we can type check every pointer operation.
This also type-checks that direct memory hashing is within
the valid range of a struct value.

We add test cases that previously caused deephash to panic,
but now pass.

Performance:

	name              old time/op    new time/op    delta
	Hash              14.1µs ± 1%    14.1µs ± 1%    ~     (p=0.590 n=10+9)
	HashPacketFilter  2.53µs ± 2%    2.44µs ± 1%  -3.79%  (p=0.000 n=9+10)
	TailcfgNode       1.45µs ± 1%    1.43µs ± 0%  -1.36%  (p=0.000 n=9+9)
	HashArray         318ns ± 2%     318ns ± 2%    ~      (p=0.541 n=10+10)
	HashMapAcyclic    32.9µs ± 1%    31.6µs ± 1%  -4.16%  (p=0.000 n=10+9)

There is a slight performance gain due to the use of unsafe.Pointer
over reflect.Value methods. Also, passing an unsafe.Pointer (1 word)
on the stack is cheaper than passing a reflect.Value (3 words).

Performance gains are diminishing since SHA-256 hashing now dominates the runtime.

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
This commit is contained in:
Joe Tsai
2022-08-27 12:30:35 -07:00
committed by GitHub
parent e0c5ac1f02
commit 31bf3874d6
5 changed files with 408 additions and 162 deletions

View File

@@ -20,6 +20,7 @@ import (
"testing/quick"
"time"
qt "github.com/frankban/quicktest"
"go4.org/mem"
"go4.org/netipx"
"tailscale.com/tailcfg"
@@ -572,13 +573,13 @@ func TestGetTypeHasher(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
rv := reflect.ValueOf(tt.val)
va := newAddressableValue(rv.Type())
va := reflect.New(rv.Type()).Elem()
va.Set(rv)
fn := getTypeInfo(va.Type()).hasher()
hb := &hashBuffer{Hash: sha256.New()}
h := new(hasher)
h.Block512.Hash = hb
fn(h, va)
fn(h, pointerOf(va.Addr()))
const ptrSize = 32 << uintptr(^uintptr(0)>>63)
if tt.out32 != "" && ptrSize == 32 {
tt.out = tt.out32
@@ -591,6 +592,90 @@ func TestGetTypeHasher(t *testing.T) {
}
}
func TestMapCycle(t *testing.T) {
type M map[string]M
c := qt.New(t)
a := make(M) // cylic graph of 1 node
a["self"] = a
b := make(M) // cylic graph of 1 node
b["self"] = b
ha := Hash(a)
hb := Hash(b)
c.Assert(ha, qt.Equals, hb)
c1 := make(M) // cyclic graph of 2 nodes
c2 := make(M) // cyclic graph of 2 nodes
c1["peer"] = c2
c2["peer"] = c1
hc1 := Hash(c1)
hc2 := Hash(c2)
c.Assert(hc1, qt.Equals, hc2)
c.Assert(ha, qt.Not(qt.Equals), hc1)
c.Assert(hb, qt.Not(qt.Equals), hc2)
c3 := make(M) // graph of 1 node pointing to cyclic graph of 2 nodes
c3["child"] = c1
hc3 := Hash(c3)
c.Assert(hc1, qt.Not(qt.Equals), hc3)
}
func TestPointerCycle(t *testing.T) {
type P *P
c := qt.New(t)
a := new(P) // cyclic graph of 1 node
*a = a
b := new(P) // cyclic graph of 1 node
*b = b
ha := Hash(&a)
hb := Hash(&b)
c.Assert(ha, qt.Equals, hb)
c1 := new(P) // cyclic graph of 2 nodes
c2 := new(P) // cyclic graph of 2 nodes
*c1 = c2
*c2 = c1
hc1 := Hash(&c1)
hc2 := Hash(&c2)
c.Assert(hc1, qt.Equals, hc2)
c.Assert(ha, qt.Not(qt.Equals), hc1)
c.Assert(hb, qt.Not(qt.Equals), hc2)
c3 := new(P) // graph of 1 node pointing to cyclic graph of 2 nodes
*c3 = c1
hc3 := Hash(&c3)
c.Assert(hc1, qt.Not(qt.Equals), hc3)
}
func TestInterfaceCycle(t *testing.T) {
type I struct{ v any }
c := qt.New(t)
a := new(I) // cyclic graph of 1 node
a.v = a
b := new(I) // cyclic graph of 1 node
b.v = b
ha := Hash(&a)
hb := Hash(&b)
c.Assert(ha, qt.Equals, hb)
c1 := new(I) // cyclic graph of 2 nodes
c2 := new(I) // cyclic graph of 2 nodes
c1.v = c2
c2.v = c1
hc1 := Hash(&c1)
hc2 := Hash(&c2)
c.Assert(hc1, qt.Equals, hc2)
c.Assert(ha, qt.Not(qt.Equals), hc1)
c.Assert(hb, qt.Not(qt.Equals), hc2)
c3 := new(I) // graph of 1 node pointing to cyclic graph of 2 nodes
c3.v = c1
hc3 := Hash(&c3)
c.Assert(hc1, qt.Not(qt.Equals), hc3)
}
var sink Sum
func BenchmarkHash(b *testing.B) {
@@ -665,11 +750,11 @@ func TestHashMapAcyclic(t *testing.T) {
ti := getTypeInfo(reflect.TypeOf(m))
for i := 0; i < 20; i++ {
v := addressableValue{reflect.ValueOf(&m).Elem()}
v := reflect.ValueOf(&m).Elem()
hb.Reset()
h := new(hasher)
h.Block512.Hash = hb
h.hashMap(v, ti, false)
h.hashMap(v, ti)
h.sum()
if got[string(hb.B)] {
continue
@@ -689,9 +774,9 @@ func TestPrintArray(t *testing.T) {
hb := &hashBuffer{Hash: sha256.New()}
h := new(hasher)
h.Block512.Hash = hb
v := addressableValue{reflect.ValueOf(&x).Elem()}
ti := getTypeInfo(v.Type())
ti.hasher()(h, v)
va := reflect.ValueOf(&x).Elem()
ti := getTypeInfo(va.Type())
ti.hasher()(h, pointerOf(va.Addr()))
h.sum()
const want = "\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x1f"
if got := hb.B; string(got) != want {
@@ -707,15 +792,15 @@ func BenchmarkHashMapAcyclic(b *testing.B) {
}
hb := &hashBuffer{Hash: sha256.New()}
v := addressableValue{reflect.ValueOf(&m).Elem()}
ti := getTypeInfo(v.Type())
va := reflect.ValueOf(&m).Elem()
ti := getTypeInfo(va.Type())
h := new(hasher)
h.Block512.Hash = hb
for i := 0; i < b.N; i++ {
h.Reset()
h.hashMap(v, ti, false)
h.hashMap(va, ti)
}
}