perf: do header clone, augment and sort once in http2 encodeHeaders

header.go

@@ -223,6 +223,10 @@ func (h Header) SortedKeyValues(exclude map[string]bool) (kvs []HeaderKeyValues,
 		mutex.RUnlock()
 	}
 	hs.kvs = kvs
+	// Reset any order left on the sorter by a previous SortedKeyValuesBy
+	// call, otherwise a pooled sorter sorts by the stale order instead of
+	// lexicographically.
+	hs.order = nil
 	sort.Sort(hs)
 	return kvs, hs
 }

header_test.go

@@ -359,3 +359,28 @@ func TestHTTP1HeaderOrder(t *testing.T) {
 		t.Fatalf("got:\n%swant:\n%s", buf.String(), expected)
 	}
 }
+
+func TestHeaderSorterPoolReuse(t *testing.T) {
+	// A sorter used by SortedKeyValuesBy keeps its order map. When it is
+	// pulled from the pool again by SortedKeyValues (no order), the stale
+	// order must not be used.
+	ordered := Header{
+		"Zebra": {"1"},
+		"Apple": {"2"},
+	}
+	// Deliberately the reverse of lexicographic order.
+	_, hs := ordered.SortedKeyValuesBy(map[string]int{"zebra": 0, "apple": 1}, nil)
+	headerSorterPool.Put(hs)
+
+	plain := Header{
+		"Apple":  {"1"},
+		"Banana": {"2"},
+		"Zebra":  {"3"},
+	}
+	kvs, _ := plain.SortedKeyValues(nil)
+	for i := 1; i < len(kvs); i++ {
+		if kvs[i-1].Key > kvs[i].Key {
+			t.Fatalf("keys not sorted lexicographically: %q before %q", kvs[i-1].Key, kvs[i].Key)
+		}
+	}
+}

http2/transport_test.go

@@ -1060,7 +1060,9 @@ const (
 )
 
 // Test all 36 combinations of response frame orders:
-//    (3 ways of 100-continue) * (2 ways of headers) * (2 ways of data) * (3 ways of trailers):func TestTransportResponsePattern_00f0(t *testing.T) { testTransportResponsePattern(h0, h1, false, h0) }
+//
+//	(3 ways of 100-continue) * (2 ways of headers) * (2 ways of data) * (3 ways of trailers):func TestTransportResponsePattern_00f0(t *testing.T) { testTransportResponsePattern(h0, h1, false, h0) }
+//
 // Generated by http://play.golang.org/p/SScqYKJYXd
 func TestTransportResPattern_c0h1d0t0(t *testing.T) { testTransportResPattern(t, f0, f1, d0, f0) }
 func TestTransportResPattern_c0h1d0t1(t *testing.T) { testTransportResPattern(t, f0, f1, d0, f1) }
@@ -1480,8 +1482,9 @@ func testInvalidTrailer(t *testing.T, trailers headerType, wantErr error, writeT
 }
 
 // headerListSize returns the HTTP2 header list size of h.
-//   http://httpwg.org/specs/rfc7540.html#SETTINGS_MAX_HEADER_LIST_SIZE
-//   http://httpwg.org/specs/rfc7540.html#MaxHeaderBlock
+//
+//	http://httpwg.org/specs/rfc7540.html#SETTINGS_MAX_HEADER_LIST_SIZE
+//	http://httpwg.org/specs/rfc7540.html#MaxHeaderBlock
 func headerListSize(h http.Header) (size uint32) {
 	for k, vv := range h {
 		for _, v := range vv {
@@ -4717,6 +4720,221 @@ func BenchmarkClientResponseHeaders(b *testing.B) {
 	b.Run("1000 Headers", func(b *testing.B) { benchSimpleRoundTrip(b, 0, 1000) })
 }
 
+// BenchmarkClientConnEncodeHeaders measures request header encoding directly,
+// without a round trip, in the two shapes that matter: with a Header-Order:
+// key (ordered sort) and without one (lexicographic sort).
+// TestClientConnEncodeHeadersOrder pins the exact header sequence
+// encodeHeaders emits, across every combination of the two ordering keys
+// being present or absent, so that changes to how the headers are prepared
+// cannot alter what goes on the wire.
+func TestClientConnEncodeHeadersOrder(t *testing.T) {
+	tests := []struct {
+		name          string
+		pseudoOrder   []string // Transport.PseudoHeaderOrder
+		header        http.Header
+		method        string
+		addGzipHeader bool
+		trailers      string
+		contentLength int64
+		want          []string
+	}{
+		{
+			// Neither ordering key, and no order on the Transport either.
+			// No pseudo-headers are emitted at all; that is pre-existing
+			// behavior on this path, unchanged by header preparation.
+			name:          "no order keys and no transport order",
+			header:        http.Header{"B-Two": {"2"}, "A-One": {"1"}},
+			addGzipHeader: true,
+			contentLength: -1,
+			want: []string{
+				"a-one: 1", "b-two: 2",
+				"accept-encoding: gzip, deflate, br",
+				"user-agent: " + defaultUserAgent,
+			},
+		},
+		{
+			// No ordering keys on the request, so the regular headers sort
+			// lexicographically and the Transport supplies the pseudo order.
+			name:          "no order keys with transport pseudo order",
+			pseudoOrder:   []string{":method", ":authority", ":scheme", ":path"},
+			header:        http.Header{"Zeta": {"z"}, "Alpha": {"a"}, "Mid": {"m1", "m2"}},
+			addGzipHeader: true,
+			contentLength: -1,
+			want: []string{
+				":method: GET", ":authority: www.example.org", ":scheme: https", ":path: /p",
+				"alpha: a", "mid: m1", "mid: m2", "zeta: z",
+				"accept-encoding: gzip, deflate, br",
+				"user-agent: " + defaultUserAgent,
+			},
+		},
+		{
+			// Pseudo order only: regular headers still sort lexicographically.
+			name: "pheader order only",
+			header: http.Header{
+				"Zeta": {"z"}, "Alpha": {"a"},
+				http.PHeaderOrderKey: {":method", ":path", ":authority", ":scheme"},
+			},
+			contentLength: -1,
+			want: []string{
+				":method: GET", ":path: /p", ":authority: www.example.org", ":scheme: https",
+				"alpha: a", "zeta: z",
+				"user-agent: " + defaultUserAgent,
+			},
+		},
+		{
+			// Header order only, with keys both in and out of the list. Keys
+			// absent from the list follow, lexicographically.
+			name:        "header order only, some keys unlisted",
+			pseudoOrder: []string{":method", ":authority", ":scheme", ":path"},
+			header: http.Header{
+				"In-Order": {"x"}, "Also-Order": {"y"},
+				"Zeta": {"z"}, "Alpha": {"a"},
+				http.HeaderOrderKey: {"in-order", "also-order"},
+			},
+			contentLength: -1,
+			want: []string{
+				":method: GET", ":authority: www.example.org", ":scheme: https", ":path: /p",
+				"in-order: x", "also-order: y", "alpha: a", "zeta: z",
+				"user-agent: " + defaultUserAgent,
+			},
+		},
+		{
+			name: "both order keys, with cookie splitting and content length",
+			header: http.Header{
+				"Cookie":             {"a=1; b=2;c=3"},
+				"User-Agent":         {"ua"},
+				"Content-Type":       {"application/json"},
+				http.HeaderOrderKey:  {"user-agent", "cookie", "content-type"},
+				http.PHeaderOrderKey: {":method", ":authority", ":scheme", ":path"},
+			},
+			method:        "POST",
+			contentLength: 7,
+			want: []string{
+				":method: POST", ":authority: www.example.org", ":scheme: https", ":path: /p",
+				"user-agent: ua", "cookie: a=1", "cookie: b=2", "cookie: c=3",
+				"content-type: application/json",
+				"content-length: 7",
+			},
+		},
+		{
+			// Connection-specific headers and Host are dropped, an empty
+			// value slice is skipped, and a multi-value User-Agent is cut
+			// to one - none of which depends on an ordering key.
+			name:        "no order keys, filtered and skipped headers",
+			pseudoOrder: []string{":method", ":authority", ":scheme", ":path"},
+			header: http.Header{
+				"Connection": {"keep-alive"}, "Upgrade": {"h2c"},
+				"Transfer-Encoding": {"chunked"}, "Host": {"other.example"},
+				"Skipped":    {},
+				"User-Agent": {"first", "second"},
+				"X-Kept":     {"yes"},
+			},
+			contentLength: -1,
+			want: []string{
+				":method: GET", ":authority: www.example.org", ":scheme: https", ":path: /p",
+				"user-agent: first", "x-kept: yes",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			u, err := url.Parse("https://www.example.org/p")
+			if err != nil {
+				t.Fatal(err)
+			}
+			method := tt.method
+			if method == "" {
+				method = "GET"
+			}
+			req := &http.Request{Method: method, URL: u, Header: tt.header}
+			cc := &ClientConn{
+				peerMaxHeaderListSize: 1 << 30,
+				t:                     &Transport{PseudoHeaderOrder: tt.pseudoOrder},
+			}
+			cc.henc = hpack.NewEncoder(&cc.hbuf)
+
+			cc.mu.Lock()
+			hdrs, err := cc.encodeHeaders(req, tt.addGzipHeader, tt.trailers, tt.contentLength)
+			cc.mu.Unlock()
+			if err != nil {
+				t.Fatalf("encodeHeaders: %v", err)
+			}
+
+			var got []string
+			dec := hpack.NewDecoder(initialHeaderTableSize, func(f hpack.HeaderField) {
+				got = append(got, f.Name+": "+f.Value)
+			})
+			if _, err := dec.Write(hdrs); err != nil {
+				t.Fatalf("hpack decode: %v", err)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("emitted headers:\n got: %q\nwant: %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func BenchmarkClientConnEncodeHeaders(b *testing.B) {
+	run := func(b *testing.B, h http.Header) {
+		b.ReportAllocs()
+		u, err := url.Parse("https://www.example.org/some/path?q=1")
+		if err != nil {
+			b.Fatal(err)
+		}
+		req := &http.Request{Method: "GET", URL: u, Header: h}
+		cc := &ClientConn{peerMaxHeaderListSize: 10 << 20, t: &Transport{}}
+		cc.henc = hpack.NewEncoder(&cc.hbuf)
+		cc.mu.Lock()
+		defer cc.mu.Unlock()
+		b.ResetTimer()
+		for i := 0; i < b.N; i++ {
+			if _, err := cc.encodeHeaders(req, true, "", -1); err != nil {
+				b.Fatal(err)
+			}
+		}
+	}
+
+	b.Run("Ordered", func(b *testing.B) {
+		run(b, http.Header{
+			"sec-ch-ua":        {"\"Chromium\";v=\"124\", \"Google Chrome\";v=\"124\""},
+			"accept":           {"*/*"},
+			"x-requested-with": {"XMLHttpRequest"},
+			"sec-ch-ua-mobile": {"?0"},
+			"user-agent":       {"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36"},
+			"content-type":     {"application/json"},
+			"origin":           {"https://www.example.org"},
+			"sec-fetch-site":   {"same-origin"},
+			"sec-fetch-mode":   {"cors"},
+			"sec-fetch-dest":   {"empty"},
+			"accept-language":  {"en-US,en;q=0.9"},
+			"accept-encoding":  {"gzip, deflate, br"},
+			"referer":          {"https://www.example.org/x"},
+			"cookie":           {"a=1; b=2; c=3"},
+			http.HeaderOrderKey: {
+				"sec-ch-ua", "accept", "x-requested-with", "sec-ch-ua-mobile",
+				"user-agent", "content-type", "origin", "sec-fetch-site",
+				"sec-fetch-mode", "sec-fetch-dest", "referer", "cookie",
+				"accept-encoding", "accept-language",
+			},
+			http.PHeaderOrderKey: {":method", ":authority", ":scheme", ":path"},
+		})
+	})
+
+	b.Run("Unordered", func(b *testing.B) {
+		run(b, http.Header{
+			"Accept":          {"*/*"},
+			"User-Agent":      {"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36"},
+			"Content-Type":    {"application/json"},
+			"Origin":          {"https://www.example.org"},
+			"Accept-Language": {"en-US,en;q=0.9"},
+			"Accept-Encoding": {"gzip, deflate, br"},
+			"Referer":         {"https://www.example.org/x"},
+			"Cookie":          {"a=1; b=2; c=3"},
+		})
+	})
+}
+
 func activeStreams(cc *ClientConn) int {
 	cc.mu.Lock()
 	defer cc.mu.Unlock()