From 4876518f9e71663000c348837735820161a42df7 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 22 Mar 2016 02:00:46 +0000 Subject: [PATCH] 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 --- http2/http2.go | 34 ++++++++++++++++++++++++++++++++++ http2/http2_test.go | 24 ++++++++++++++++++++++++ http2/server.go | 8 ++++++-- http2/write.go | 13 ++++++------- 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/http2/http2.go b/http2/http2.go index 4c5e11ac..0529b63e 100644 --- a/http2/http2.go +++ b/http2/http2.go @@ -23,6 +23,7 @@ import ( "io" "net/http" "os" + "sort" "strconv" "strings" "sync" @@ -427,3 +428,36 @@ var isTokenTable = [127]bool{ type connectionStater interface { 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 +} diff --git a/http2/http2_test.go b/http2/http2_test.go index 0a4da46a..34dcb137 100644 --- a/http2/http2_test.go +++ b/http2/http2_test.go @@ -172,3 +172,27 @@ func cleanDate(res *http.Response) { 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) + } +} diff --git a/http2/server.go b/http2/server.go index 74ec4a3e..1e6980c3 100644 --- a/http2/server.go +++ b/http2/server.go @@ -51,7 +51,6 @@ import ( "os" "reflect" "runtime" - "sort" "strconv" "strings" "sync" @@ -2026,7 +2025,12 @@ func (rws *responseWriterState) promoteUndeclaredTrailers() { rws.declareTrailer(trailerKey) 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() { diff --git a/http2/write.go b/http2/write.go index 5297a4bf..0143b24c 100644 --- a/http2/write.go +++ b/http2/write.go @@ -9,7 +9,6 @@ import ( "fmt" "log" "net/http" - "sort" "time" "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) { - // TODO: garbage. pool sorters like http1? hot path for 1 key? 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 { vv := h[k]