Skip to content

Commit

Permalink
http2: mark some structs as non-comparable
Browse files Browse the repository at this point in the history
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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
  • Loading branch information
bradfitz committed May 1, 2020
1 parent ff2c4b7 commit e0ff5e5
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 12 deletions.
2 changes: 2 additions & 0 deletions http2/client_conn_pool.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions http2/flow.go
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions http2/frame_test.go
Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand Down
7 changes: 7 additions & 0 deletions http2/hpack/huffman.go
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions http2/http2.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -313,6 +314,7 @@ func bodyAllowedForStatus(status int) bool {
}

type httpError struct {
_ incomparable
msg string
timeout bool
}
Expand Down Expand Up @@ -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()
6 changes: 4 additions & 2 deletions http2/server.go
Expand Up @@ -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
}
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
12 changes: 4 additions & 8 deletions http2/transport.go
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -1616,6 +1609,7 @@ func (cc *ClientConn) writeHeader(name, value string) {
}

type resAndError struct {
_ incomparable
res *http.Response
err error
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e0ff5e5

Please sign in to comment.