internal/deephash: re-use MapIter

name              old time/op    new time/op    delta
Hash-8              12.4µs ± 0%    12.4µs ± 0%    -0.33%  (p=0.002 n=10+9)
HashMapAcyclic-8    21.2µs ± 0%    21.3µs ± 0%    +0.45%  (p=0.000 n=8+8)

name              old alloc/op   new alloc/op   delta
Hash-8                793B ± 0%      408B ± 0%   -48.55%  (p=0.000 n=10+10)
HashMapAcyclic-8      128B ± 0%        0B       -100.00%  (p=0.000 n=10+10)

name              old allocs/op  new allocs/op  delta
Hash-8                9.00 ± 0%      6.00 ± 0%   -33.33%  (p=0.000 n=10+10)
HashMapAcyclic-8      1.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)

Depends on https://github.com/golang/go/issues/46293.

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

internal/deephash/deephash.go

@@ -197,6 +197,7 @@ type mapHasher struct {
 	s256 hash.Hash         // sha256 hash.Hash
 	bw   *bufio.Writer     // to hasher into ebuf
 	val  valueCache        // re-usable values for map iteration
+	iter *reflect.MapIter  // re-usable map iterator
 }
 
 func (mh *mapHasher) Reset() {
@@ -226,6 +227,7 @@ var mapHasherPool = &sync.Pool{
 		mh.s256 = sha256.New()
 		mh.bw = bufio.NewWriter(mh.s256)
 		mh.val = make(valueCache)
+		mh.iter = new(reflect.MapIter)
 		return mh
 	},
 }
@@ -248,7 +250,8 @@ func hashMapAcyclic(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool,
 	mh := mapHasherPool.Get().(*mapHasher)
 	defer mapHasherPool.Put(mh)
 	mh.Reset()
-	iter := v.MapRange()
+	iter := mapIter(mh.iter, v)
+	defer mapIter(mh.iter, reflect.Value{}) // avoid pinning v from mh.iter when we return
 	k := mh.val.get(v.Type().Key())
 	e := mh.val.get(v.Type().Elem())
 	for iter.Next() {

internal/deephash/mapiter.go

@@ -23,3 +23,15 @@ func iterKey(iter *reflect.MapIter, _ reflect.Value) reflect.Value {
 func iterVal(iter *reflect.MapIter, _ reflect.Value) reflect.Value {
 	return iter.Value()
 }
+
+// mapIter returns a map iterator for mapVal.
+// scratch is a re-usable reflect.MapIter.
+// mapIter may re-use scratch and return it,
+// or it may allocate and return a new *reflect.MapIter.
+// If mapVal is the zero reflect.Value, mapIter may return nil.
+func mapIter(_ *reflect.MapIter, mapVal reflect.Value) *reflect.MapIter {
+	if !mapVal.IsValid() {
+		return nil
+	}
+	return mapVal.MapRange()
+}

internal/deephash/mapiter_future.go

@@ -25,3 +25,18 @@ func iterVal(iter *reflect.MapIter, scratch reflect.Value) reflect.Value {
 	iter.SetValue(scratch)
 	return scratch
 }
+
+// mapIter returns a map iterator for mapVal.
+// scratch is a re-usable reflect.MapIter.
+// mapIter may re-use scratch and return it,
+// or it may allocate and return a new *reflect.MapIter.
+// If mapVal is the zero reflect.Value, mapIter may return nil.
+func mapIter(scratch *reflect.MapIter, mapVal reflect.Value) *reflect.MapIter {
+	scratch.Reset(mapVal) // always Reset, to allow the caller to avoid pinning memory
+	if !mapVal.IsValid() {
+		// Returning scratch would also be OK.
+		// Do this for consistency with the non-optimized version.
+		return nil
+	}
+	return scratch
+}