Skip to content

Commit

Permalink
Improve unpackString performance (#1011)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
tmthrgd authored and miekg committed Sep 22, 2019
1 parent ba5bfd0 commit b733ad8
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 13 deletions.
21 changes: 17 additions & 4 deletions msg_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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('\\')
Expand All @@ -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
}

Expand Down
35 changes: 26 additions & 9 deletions msg_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
})
b.Run("Unescaped", func(b *testing.B) {
msg := []byte("\x00large.example.com")
msg[0] = byte(len(msg) - 1)

if want := `abcdef\015\\\"ghi\004mmm`; want != got {
b.Errorf("expected %q, got %q", want, got)
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)
}
}
}
})
}

0 comments on commit b733ad8

Please sign in to comment.