http2: mark some structs as non-comparable

Reduces binary size by not generating eq algs. Also, remove the badStringError type that only had one use, and was just copied from net/http where it's also not used much. Updates golang/go#38782 Change-Id: I56bddde0bb500109e2c18bb1419e8a920a5bebf9 Reviewed-on: https://go-review.googlesource.com/c/net/+/231119 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
golang · May 1, 2020 · e0ff5e5 · e0ff5e5
1 parent ff2c4b7
commit e0ff5e5
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 12 deletions.
diff --git a/http2/client_conn_pool.go b/http2/client_conn_pool.go
@@ -107,6 +107,7 @@ func (p *clientConnPool) getClientConn(req *http.Request, addr string, dialOnMis
 
 // dialCall is an in-flight Transport dial call to a host.
 type dialCall struct {
+	_    incomparable
 	p    *clientConnPool
 	done chan struct{} // closed when done
 	res  *ClientConn   // valid after done is closed
@@ -180,6 +181,7 @@ func (p *clientConnPool) addConnIfNeeded(key string, t *Transport, c *tls.Conn)
 }
 
 type addConnCall struct {
+	_    incomparable
 	p    *clientConnPool
 	done chan struct{} // closed when done
 	err  error

diff --git a/http2/flow.go b/http2/flow.go
@@ -8,6 +8,8 @@ package http2
 
 // flow is the flow control window's size.
 type flow struct {
+	_ incomparable
+
 	// n is the number of DATA bytes we're allowed to send.
 	// A flow is kept both on a conn and a per-stream.
 	n int32

diff --git a/http2/frame_test.go b/http2/frame_test.go
@@ -160,7 +160,7 @@ func TestWriteDataPadded(t *testing.T) {
 		}
 		got := f.Header()
 		tt.wantHeader.valid = true
-		if got != tt.wantHeader {
+		if !got.Equal(tt.wantHeader) {
 			t.Errorf("%d. read %+v; want %+v", i, got, tt.wantHeader)
 			continue
 		}
@@ -171,6 +171,14 @@ func TestWriteDataPadded(t *testing.T) {
 	}
 }
 
+func (fh FrameHeader) Equal(b FrameHeader) bool {
+	return fh.valid == b.valid &&
+		fh.Type == b.Type &&
+		fh.Flags == b.Flags &&
+		fh.Length == b.Length &&
+		fh.StreamID == b.StreamID
+}
+
 func TestWriteHeaders(t *testing.T) {
 	tests := []struct {
 		name      string
@@ -595,7 +603,7 @@ func TestReadFrameHeader(t *testing.T) {
 			continue
 		}
 		tt.want.valid = true
-		if got != tt.want {
+		if !got.Equal(tt.want) {
 			t.Errorf("%d. readFrameHeader(%q) = %+v; want %+v", i, tt.in, got, tt.want)
 		}
 	}

diff --git a/http2/hpack/huffman.go b/http2/hpack/huffman.go
@@ -105,7 +105,14 @@ func huffmanDecode(buf *bytes.Buffer, maxLen int, v []byte) error {
 	return nil
 }
 
+// incomparable is a zero-width, non-comparable type. Adding it to a struct
+// makes that struct also non-comparable, and generally doesn't add
+// any size (as long as it's first).
+type incomparable [0]func()
+
 type node struct {
+	_ incomparable
+
 	// children is non-nil for internal nodes
 	children *[256]*node
 

diff --git a/http2/http2.go b/http2/http2.go
@@ -241,6 +241,7 @@ func (cw closeWaiter) Wait() {
 // Its buffered writer is lazily allocated as needed, to minimize
 // idle memory usage with many connections.
 type bufferedWriter struct {
+	_  incomparable
 	w  io.Writer     // immutable
 	bw *bufio.Writer // non-nil when data is buffered
 }
@@ -313,6 +314,7 @@ func bodyAllowedForStatus(status int) bool {
 }
 
 type httpError struct {
+	_       incomparable
 	msg     string
 	timeout bool
 }
@@ -376,3 +378,8 @@ func (s *sorter) SortStrings(ss []string) {
 func validPseudoPath(v string) bool {
 	return (len(v) > 0 && v[0] == '/') || v == "*"
 }
+
+// incomparable is a zero-width, non-comparable type. Adding it to a struct
+// makes that struct also non-comparable, and generally doesn't add
+// any size (as long as it's first).
+type incomparable [0]func()
diff --git a/http2/server.go b/http2/server.go
@@ -761,6 +761,7 @@ func (sc *serverConn) readFrames() {
 
 // frameWriteResult is the message passed from writeFrameAsync to the serve goroutine.
 type frameWriteResult struct {
+	_   incomparable
 	wr  FrameWriteRequest // what was written (or attempted)
 	err error             // result of the writeFrame call
 }
@@ -771,7 +772,7 @@ type frameWriteResult struct {
 // serverConn.
 func (sc *serverConn) writeFrameAsync(wr FrameWriteRequest) {
 	err := wr.write.writeFrame(sc)
-	sc.wroteFrameCh <- frameWriteResult{wr, err}
+	sc.wroteFrameCh <- frameWriteResult{wr: wr, err: err}
 }
 
 func (sc *serverConn) closeAllStreamsOnConnClose() {
@@ -1161,7 +1162,7 @@ func (sc *serverConn) startFrameWrite(wr FrameWriteRequest) {
 	if wr.write.staysWithinBuffer(sc.bw.Available()) {
 		sc.writingFrameAsync = false
 		err := wr.write.writeFrame(sc)
-		sc.wroteFrame(frameWriteResult{wr, err})
+		sc.wroteFrame(frameWriteResult{wr: wr, err: err})
 	} else {
 		sc.writingFrameAsync = true
 		go sc.writeFrameAsync(wr)
@@ -2275,6 +2276,7 @@ func (sc *serverConn) sendWindowUpdate32(st *stream, n int32) {
 // requestBody is the Handler's Request.Body type.
 // Read and Close may be called concurrently.
 type requestBody struct {
+	_             incomparable
 	stream        *stream
 	conn          *serverConn
 	closed        bool  // for use by Close only

diff --git a/http2/transport.go b/http2/transport.go
@@ -916,7 +916,7 @@ func commaSeparatedTrailers(req *http.Request) (string, error) {
 		k = http.CanonicalHeaderKey(k)
 		switch k {
 		case "Transfer-Encoding", "Trailer", "Content-Length":
-			return "", &badStringError{"invalid Trailer key", k}
+			return "", fmt.Errorf("invalid Trailer key %q", k)
 		}
 		keys = append(keys, k)
 	}
@@ -1394,13 +1394,6 @@ func (cs *clientStream) awaitFlowControl(maxBytes int) (taken int32, err error)
 	}
 }
 
-type badStringError struct {
-	what string
-	str  string
-}
-
-func (e *badStringError) Error() string { return fmt.Sprintf("%s %q", e.what, e.str) }
-
 // requires cc.mu be held.
 func (cc *ClientConn) encodeHeaders(req *http.Request, addGzipHeader bool, trailers string, contentLength int64) ([]byte, error) {
 	cc.hbuf.Reset()
@@ -1616,6 +1609,7 @@ func (cc *ClientConn) writeHeader(name, value string) {
 }
 
 type resAndError struct {
+	_   incomparable
 	res *http.Response
 	err error
 }
@@ -1663,6 +1657,7 @@ func (cc *ClientConn) streamByID(id uint32, andRemove bool) *clientStream {
 
 // clientConnReadLoop is the state owned by the clientConn's frame-reading readLoop.
 type clientConnReadLoop struct {
+	_             incomparable
 	cc            *ClientConn
 	closeWhenIdle bool
 }
@@ -2479,6 +2474,7 @@ func (rt erringRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
 // gzipReader wraps a response body so it can lazily
 // call gzip.NewReader on the first call to Read
 type gzipReader struct {
+	_    incomparable
 	body io.ReadCloser // underlying Response.Body
 	zr   *gzip.Reader  // lazily-initialized gzip reader
 	zerr error         // sticky error