From b733ad8671876b67c54799a958a076bc95975b99 Mon Sep 17 00:00:00 2001
From: Tom Thorogood <me+github@tomthorogood.co.uk>
Date: Sun, 22 Sep 2019 17:29:05 +0930
Subject: [PATCH] Improve unpackString performance (#1011)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I'm not convinced this is really worth doing, but it does show a
performance improvement.

name                       old time/op    new time/op    delta
UnpackString/Escaped-12      83.7ns ± 7%    78.2ns ± 3%   -6.50%  (p=0.000 n=10+9)
UnpackString/Unescaped-12    57.8ns ± 9%    50.4ns ±13%  -12.74%  (p=0.000 n=10+10)

name                       old alloc/op   new alloc/op   delta
UnpackString/Escaped-12       48.0B ± 0%     32.0B ± 0%  -33.33%  (p=0.000 n=10+10)
UnpackString/Unescaped-12     32.0B ± 0%     32.0B ± 0%     ~     (all equal)

name                       old allocs/op  new allocs/op  delta
UnpackString/Escaped-12        2.00 ± 0%      1.00 ± 0%  -50.00%  (p=0.000 n=10+10)
UnpackString/Unescaped-12      1.00 ± 0%      1.00 ± 0%     ~     (all equal)
---
 msg_helpers.go      | 21 +++++++++++++++++----
 msg_helpers_test.go | 37 +++++++++++++++++++++++++++----------
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/msg_helpers.go b/msg_helpers.go
index cb4ae764..66674339 100644
--- a/msg_helpers.go
+++ b/msg_helpers.go
@@ -265,12 +265,25 @@ func unpackString(msg []byte, off int) (string, int, error) {
 		return "", off, &Error{err: "overflow unpacking txt"}
 	}
 	l := int(msg[off])
-	if off+l+1 > len(msg) {
+	off++
+	if off+l > len(msg) {
 		return "", off, &Error{err: "overflow unpacking txt"}
 	}
+	escapedLen := l
+	for _, b := range msg[off : off+l] {
+		switch {
+		case b == '"' || b == '\\':
+			escapedLen++
+		case b < ' ' || b > '~': // unprintable
+			escapedLen += 3 // escapeByte always returns four characters
+		}
+	}
+	if escapedLen == l { // no escaping needed
+		return string(msg[off : off+l]), off + l, nil
+	}
 	var s strings.Builder
-	s.Grow(l)
-	for _, b := range msg[off+1 : off+1+l] {
+	s.Grow(escapedLen)
+	for _, b := range msg[off : off+l] {
 		switch {
 		case b == '"' || b == '\\':
 			s.WriteByte('\\')
@@ -281,7 +294,7 @@ func unpackString(msg []byte, off int) (string, int, error) {
 			s.WriteByte(b)
 		}
 	}
-	off += 1 + l
+	off += l
 	return s.String(), off, nil
 }
 
diff --git a/msg_helpers_test.go b/msg_helpers_test.go
index ebeac84d..9b66d228 100644
--- a/msg_helpers_test.go
+++ b/msg_helpers_test.go
@@ -122,17 +122,34 @@ func TestUnpackString(t *testing.T) {
 }
 
 func BenchmarkUnpackString(b *testing.B) {
-	msg := []byte("\x00abcdef\x0f\\\"ghi\x04mmm")
-	msg[0] = byte(len(msg) - 1)
+	b.Run("Escaped", func(b *testing.B) {
+		msg := []byte("\x00abcdef\x0f\\\"ghi\x04mmm")
+		msg[0] = byte(len(msg) - 1)
 
-	for n := 0; n < b.N; n++ {
-		got, _, err := unpackString(msg, 0)
-		if err != nil {
-			b.Fatal(err)
-		}
+		for n := 0; n < b.N; n++ {
+			got, _, err := unpackString(msg, 0)
+			if err != nil {
+				b.Fatal(err)
+			}
 
-		if want := `abcdef\015\\\"ghi\004mmm`; want != got {
-			b.Errorf("expected %q, got %q", want, got)
+			if want := `abcdef\015\\\"ghi\004mmm`; want != got {
+				b.Errorf("expected %q, got %q", want, got)
+			}
 		}
-	}
+	})
+	b.Run("Unescaped", func(b *testing.B) {
+		msg := []byte("\x00large.example.com")
+		msg[0] = byte(len(msg) - 1)
+
+		for n := 0; n < b.N; n++ {
+			got, _, err := unpackString(msg, 0)
+			if err != nil {
+				b.Fatal(err)
+			}
+
+			if want := "large.example.com"; want != got {
+				b.Errorf("expected %q, got %q", want, got)
+			}
+		}
+	})
 }