strings: use sync.Pool to cache buffer

benchmark                         old ns/op    new ns/op    delta
BenchmarkByteReplacerWriteString       3596         3094  -13.96%

benchmark                        old allocs   new allocs    delta
BenchmarkByteReplacerWriteString          1            0  -100.00%

LGTM=dvyukov
R=bradfitz, dave, dvyukov
CC=golang-codereviews
https://golang.org/cl/101330053

diff --git a/src/pkg/strings/replace.go b/src/pkg/strings/replace.go
index cb9d7b1fa45b62fe9a42c57a4b82cdafef5fa952..89aca95bae101df1ea145fb29fcb9b52195adea9 100644 (file)
--- a/src/pkg/strings/replace.go
+++ b/src/pkg/strings/replace.go
@@ -4,7 +4,10 @@
 
 package strings
 
-import "io"
+import (
+       "io"
+       "sync"
+)
 
 // A Replacer replaces a list of strings with replacements.
 type Replacer struct {
@@ -451,27 +454,31 @@ func (r *byteReplacer) Replace(s string) string {
        return string(buf)
 }
 
-func (r *byteReplacer) WriteString(w io.Writer, s string) (n int, err error) {
-       // TODO(bradfitz): use io.WriteString with slices of s, avoiding allocation.
-       bufsize := 32 << 10
-       if len(s) < bufsize {
-               bufsize = len(s)
-       }
-       buf := make([]byte, bufsize)
+var bufferPool = sync.Pool{
+       New: func() interface{} {
+               b := make([]byte, 4096)
+               return &b
+       },
+}
 
+func (r *byteReplacer) WriteString(w io.Writer, s string) (n int, err error) {
+       bp := bufferPool.Get().(*[]byte)
+       buf := *bp
        for len(s) > 0 {
-               ncopy := copy(buf, s[:])
-               s = s[ncopy:]
+               ncopy := copy(buf, s)
                for i, b := range buf[:ncopy] {
                        buf[i] = r.new[b]
                }
-               wn, err := w.Write(buf[:ncopy])
+               s = s[ncopy:]
+               var wn int
+               wn, err = w.Write(buf[:ncopy])
                n += wn
                if err != nil {
-                       return n, err
+                       break
                }
        }
-       return n, nil
+       bufferPool.Put(bp)
+       return
 }
 
 // byteStringReplacer is the implementation that's used when all the

diff --git a/src/pkg/strings/replace_test.go b/src/pkg/strings/replace_test.go
index 2cb318b69d7e25884ac41788a48385c0b669c5f3..77e48b988bcefc4638a4828767cd8fd2d623526d 100644 (file)
--- a/src/pkg/strings/replace_test.go
+++ b/src/pkg/strings/replace_test.go
@@ -308,20 +308,21 @@ func TestReplacer(t *testing.T) {
        }
 }
 
+var algorithmTestCases = []struct {
+       r    *Replacer
+       want string
+}{
+       {capitalLetters, "*strings.byteReplacer"},
+       {htmlEscaper, "*strings.byteStringReplacer"},
+       {NewReplacer("12", "123"), "*strings.singleStringReplacer"},
+       {NewReplacer("1", "12"), "*strings.byteStringReplacer"},
+       {NewReplacer("", "X"), "*strings.genericReplacer"},
+       {NewReplacer("a", "1", "b", "12", "cde", "123"), "*strings.genericReplacer"},
+}
+
 // TestPickAlgorithm tests that NewReplacer picks the correct algorithm.
 func TestPickAlgorithm(t *testing.T) {
-       testCases := []struct {
-               r    *Replacer
-               want string
-       }{
-               {capitalLetters, "*strings.byteReplacer"},
-               {htmlEscaper, "*strings.byteStringReplacer"},
-               {NewReplacer("12", "123"), "*strings.singleStringReplacer"},
-               {NewReplacer("1", "12"), "*strings.byteStringReplacer"},
-               {NewReplacer("", "X"), "*strings.genericReplacer"},
-               {NewReplacer("a", "1", "b", "12", "cde", "123"), "*strings.genericReplacer"},
-       }
-       for i, tc := range testCases {
+       for i, tc := range algorithmTestCases {
                got := fmt.Sprintf("%T", tc.r.Replacer())
                if got != tc.want {
                        t.Errorf("%d. algorithm = %s, want %s", i, got, tc.want)
@@ -329,6 +330,23 @@ func TestPickAlgorithm(t *testing.T) {
        }
 }
 
+type errWriter struct{}
+
+func (errWriter) Write(p []byte) (n int, err error) {
+       return 0, fmt.Errorf("unwritable")
+}
+
+// TestWriteStringError tests that WriteString returns an error
+// received from the underlying io.Writer.
+func TestWriteStringError(t *testing.T) {
+       for i, tc := range algorithmTestCases {
+               n, err := tc.r.WriteString(errWriter{}, "abc")
+               if n != 0 || err == nil || err.Error() != "unwritable" {
+                       t.Errorf("%d. WriteStringError = %d, %v, want 0, unwritable", i, n, err)
+               }
+       }
+}
+
 // TestGenericTrieBuilding verifies the structure of the generated trie. There
 // is one node per line, and the key ending with the current line is in the
 // trie if it ends with a "+".