From 752f8c0f2fcdefb83e5a8bcbc53ff444b6fa69e4 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Mon, 10 May 2021 13:29:56 -0700
Subject: [PATCH] internal/deepprint: buffer writes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The sha256 hash writer doesn't implement WriteString.
(See https://github.com/golang/go/issues/38776.)
As a consequence, we end up converting many strings to []byte.

Wrapping a bufio.Writer around the hash writer lets us
avoid these conversions by using WriteString.

Using a bufio.Writer is, perhaps surprisingly, almost as cheap as using unsafe.
The reason is that the sha256 writer does internal buffering,
but doesn't do any when handed larger writers.
Using a bufio.Writer merely shifts the data copying from one buffer
to a different one.

Using a concrete type for Print and print cuts 10% off of the execution time.

name    old time/op    new time/op    delta
Hash-8    15.3µs ± 0%    11.5µs ± 0%  -24.84%  (p=0.000 n=10+10)

name    old alloc/op   new alloc/op   delta
Hash-8    2.82kB ± 0%    1.98kB ± 0%  -29.57%  (p=0.000 n=10+10)

name    old allocs/op  new allocs/op  delta
Hash-8       140 ± 0%        82 ± 0%  -41.43%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint.go      | 16 ++++++++++------
 internal/deepprint/deepprint_test.go |  5 -----
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/internal/deepprint/deepprint.go b/internal/deepprint/deepprint.go
index 75088d553..580b85237 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deepprint/deepprint.go
@@ -12,15 +12,18 @@
 package deepprint
 
 import (
+	"bufio"
 	"crypto/sha256"
 	"fmt"
-	"io"
 	"reflect"
 )
 
 func Hash(v ...interface{}) string {
 	h := sha256.New()
-	Print(h, v)
+	// 64 matches the chunk size in crypto/sha256/sha256.go
+	b := bufio.NewWriterSize(h, 64)
+	Print(b, v)
+	b.Flush()
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
@@ -34,11 +37,11 @@ func UpdateHash(last *string, v ...interface{}) (changed bool) {
 	return false
 }
 
-func Print(w io.Writer, v ...interface{}) {
+func Print(w *bufio.Writer, v ...interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 
-func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
+func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 	if !v.IsValid() {
 		return
 	}
@@ -58,7 +61,8 @@ func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
 		t := v.Type()
 		for i, n := 0, v.NumField(); i < n; i++ {
 			sf := t.Field(i)
-			fmt.Fprintf(w, "%s: ", sf.Name)
+			w.WriteString(sf.Name)
+			w.WriteString(": ")
 			print(w, v.Field(i), visited)
 			fmt.Fprintf(w, "\n")
 		}
@@ -88,7 +92,7 @@ func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
 		fmt.Fprintf(w, "}\n")
 
 	case reflect.String:
-		fmt.Fprintf(w, "%s", v.String())
+		w.WriteString(v.String())
 	case reflect.Bool:
 		fmt.Fprintf(w, "%v", v.Bool())
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
diff --git a/internal/deepprint/deepprint_test.go b/internal/deepprint/deepprint_test.go
index 0b2d25423..796930fb8 100644
--- a/internal/deepprint/deepprint_test.go
+++ b/internal/deepprint/deepprint_test.go
@@ -5,7 +5,6 @@
 package deepprint
 
 import (
-	"bytes"
 	"testing"
 
 	"inet.af/netaddr"
@@ -18,10 +17,6 @@ func TestDeepPrint(t *testing.T) {
 	// Mostly we're just testing that we don't panic on handled types.
 	v := getVal()
 
-	var buf bytes.Buffer
-	Print(&buf, v)
-	t.Logf("Got: %s", buf.Bytes())
-
 	hash1 := Hash(v)
 	t.Logf("hash: %v", hash1)
 	for i := 0; i < 20; i++ {