Browse Source

http2: don't make garbage when sorting things

benchmark                        old ns/op     new ns/op     delta
BenchmarkServer_GetRequest-2     259453        256050        -1.31%

benchmark                        old allocs     new allocs     delta
BenchmarkServer_GetRequest-2     24             22             -8.33%

benchmark                        old bytes     new bytes     delta
BenchmarkServer_GetRequest-2     1599          1532          -4.19%

Change-Id: Ieb11a3bd4c752567e0401bcc5e77e027cfb8063c
Reviewed-on: https://go-review.googlesource.com/20999
Reviewed-by: Andrew Gerrand <adg@golang.org>
Brad Fitzpatrick 9 years ago
parent
commit
4876518f9e
4 changed files with 70 additions and 9 deletions
  1. 34 0
      http2/http2.go
  2. 24 0
      http2/http2_test.go
  3. 6 2
      http2/server.go
  4. 6 7
      http2/write.go

+ 34 - 0
http2/http2.go

@@ -23,6 +23,7 @@ import (
 	"io"
 	"io"
 	"net/http"
 	"net/http"
 	"os"
 	"os"
+	"sort"
 	"strconv"
 	"strconv"
 	"strings"
 	"strings"
 	"sync"
 	"sync"
@@ -427,3 +428,36 @@ var isTokenTable = [127]bool{
 type connectionStater interface {
 type connectionStater interface {
 	ConnectionState() tls.ConnectionState
 	ConnectionState() tls.ConnectionState
 }
 }
+
+var sorterPool = sync.Pool{New: func() interface{} { return new(sorter) }}
+
+type sorter struct {
+	v []string // owned by sorter
+}
+
+func (s *sorter) Len() int           { return len(s.v) }
+func (s *sorter) Swap(i, j int)      { s.v[i], s.v[j] = s.v[j], s.v[i] }
+func (s *sorter) Less(i, j int) bool { return s.v[i] < s.v[j] }
+
+// Keys returns the sorted keys of h.
+//
+// The returned slice is only valid until s used again or returned to
+// its pool.
+func (s *sorter) Keys(h http.Header) []string {
+	keys := s.v[:0]
+	for k := range h {
+		keys = append(keys, k)
+	}
+	s.v = keys
+	sort.Sort(s)
+	return keys
+}
+
+func (s *sorter) SortStrings(ss []string) {
+	// Our sorter works on s.v, which sorter owners, so
+	// stash it away while we sort the user's buffer.
+	save := s.v
+	s.v = ss
+	sort.Sort(s)
+	s.v = save
+}

+ 24 - 0
http2/http2_test.go

@@ -172,3 +172,27 @@ func cleanDate(res *http.Response) {
 		d[0] = "XXX"
 		d[0] = "XXX"
 	}
 	}
 }
 }
+
+func TestSorterPoolAllocs(t *testing.T) {
+	ss := []string{"a", "b", "c"}
+	h := http.Header{
+		"a": nil,
+		"b": nil,
+		"c": nil,
+	}
+	sorter := new(sorter)
+
+	if allocs := testing.AllocsPerRun(100, func() {
+		sorter.SortStrings(ss)
+	}); allocs >= 1 {
+		t.Logf("SortStrings allocs = %v; want <1", allocs)
+	}
+
+	if allocs := testing.AllocsPerRun(5, func() {
+		if len(sorter.Keys(h)) != 3 {
+			t.Fatal("wrong result")
+		}
+	}); allocs > 0 {
+		t.Logf("Keys allocs = %v; want <1", allocs)
+	}
+}

+ 6 - 2
http2/server.go

@@ -51,7 +51,6 @@ import (
 	"os"
 	"os"
 	"reflect"
 	"reflect"
 	"runtime"
 	"runtime"
-	"sort"
 	"strconv"
 	"strconv"
 	"strings"
 	"strings"
 	"sync"
 	"sync"
@@ -2026,7 +2025,12 @@ func (rws *responseWriterState) promoteUndeclaredTrailers() {
 		rws.declareTrailer(trailerKey)
 		rws.declareTrailer(trailerKey)
 		rws.handlerHeader[http.CanonicalHeaderKey(trailerKey)] = vv
 		rws.handlerHeader[http.CanonicalHeaderKey(trailerKey)] = vv
 	}
 	}
-	sort.Strings(rws.trailers)
+
+	if len(rws.trailers) > 1 {
+		sorter := sorterPool.Get().(*sorter)
+		sorter.SortStrings(rws.trailers)
+		sorterPool.Put(sorter)
+	}
 }
 }
 
 
 func (w *responseWriter) Flush() {
 func (w *responseWriter) Flush() {

+ 6 - 7
http2/write.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"fmt"
 	"log"
 	"log"
 	"net/http"
 	"net/http"
-	"sort"
 	"time"
 	"time"
 
 
 	"golang.org/x/net/http2/hpack"
 	"golang.org/x/net/http2/hpack"
@@ -230,13 +229,13 @@ func (wu writeWindowUpdate) writeFrame(ctx writeContext) error {
 }
 }
 
 
 func encodeHeaders(enc *hpack.Encoder, h http.Header, keys []string) {
 func encodeHeaders(enc *hpack.Encoder, h http.Header, keys []string) {
-	// TODO: garbage. pool sorters like http1? hot path for 1 key?
 	if keys == nil {
 	if keys == nil {
-		keys = make([]string, 0, len(h))
-		for k := range h {
-			keys = append(keys, k)
-		}
-		sort.Strings(keys)
+		sorter := sorterPool.Get().(*sorter)
+		// Using defer here, since the returned keys from the
+		// sorter.Keys method is only valid until the sorter
+		// is returned:
+		defer sorterPool.Put(sorter)
+		keys = sorter.Keys(h)
 	}
 	}
 	for _, k := range keys {
 	for _, k := range keys {
 		vv := h[k]
 		vv := h[k]