x/net/http2: low tcp segment utilization

[detailyang]

What version of Go are you using (go version)?

$ go version
go version go1.14.2 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/detailyang/Library/Caches/go-build"
GOENV="/Users/detailyang/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="**.baidu.com**"
GONOSUMDB="*"
GOOS="darwin"
GOPATH="/Users/detailyang/go"
GOPRIVATE=""
GOPROXY="https://goproxy.baidu.com,https://goproxy.cn,direct"
GOROOT="/usr/local/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/n3/g3fv953s435gqv0x7557ftyc0000gp/T/go-build515636404=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

start an http2 echo server then write a response

What did you expect to see?

I'm expecting the response Header Frame DataFrame and Header Frame with EndStream can encoding to the one TCP Segment but it's not :(

What did you see instead?

one http2 response division into 3 TCP segments which means poor performance

the related codebase is the following https://github.com/golang/net/blob/master/http2/server.go#L2393-L2499

The underlying serveConn.wantWriteFrameCh should consider how to batching fetch

func (rws *responseWriterState) writeChunk(p []byte) (n int, err error) {
    if !rws.wroteHeader {
        rws.writeHeader(200)
    }

    isHeadResp := rws.req.Method == "HEAD"
    if !rws.sentHeader {
        rws.sentHeader = true
        var ctype, clen string
        if clen = rws.snapHeader.Get("Content-Length"); clen != "" {
            rws.snapHeader.Del("Content-Length")
            clen64, err := strconv.ParseInt(clen, 10, 64)
            if err == nil && clen64 >= 0 {
                rws.sentContentLen = clen64
            } else {
                clen = ""
            }
        }
        if clen == "" && rws.handlerDone && bodyAllowedForStatus(rws.status) && (len(p) > 0 || !isHeadResp) {
            clen = strconv.Itoa(len(p))
        }
        _, hasContentType := rws.snapHeader["Content-Type"]
        // If the Content-Encoding is non-blank, we shouldn't
        // sniff the body. See Issue golang.org/issue/31753.
        ce := rws.snapHeader.Get("Content-Encoding")
        hasCE := len(ce) > 0
        if !hasCE && !hasContentType && bodyAllowedForStatus(rws.status) && len(p) > 0 {
            ctype = http.DetectContentType(p)
        }
        var date string
        if _, ok := rws.snapHeader["Date"]; !ok {
            // TODO(bradfitz): be faster here, like net/http? measure.
            date = time.Now().UTC().Format(http.TimeFormat)
        }

        for _, v := range rws.snapHeader["Trailer"] {
            foreachHeaderElement(v, rws.declareTrailer)
        }

        // "Connection" headers aren't allowed in HTTP/2 (RFC 7540, 8.1.2.2),
        // but respect "Connection" == "close" to mean sending a GOAWAY and tearing
        // down the TCP connection when idle, like we do for HTTP/1.
        // TODO: remove more Connection-specific header fields here, in addition
        // to "Connection".
        if _, ok := rws.snapHeader["Connection"]; ok {
            v := rws.snapHeader.Get("Connection")
            delete(rws.snapHeader, "Connection")
            if v == "close" {
                rws.conn.startGracefulShutdown()
            }
        }

        endStream := (rws.handlerDone && !rws.hasTrailers() && len(p) == 0) || isHeadResp
        err = rws.conn.writeHeaders(rws.stream, &writeResHeaders{
            streamID:      rws.stream.id,
            httpResCode:   rws.status,
            h:             rws.snapHeader,
            endStream:     endStream,
            contentType:   ctype,
            contentLength: clen,
            date:          date,
        })
        if err != nil {
            rws.dirty = true
            return 0, err
        }
        if endStream {
            return 0, nil
        }
    }
    if isHeadResp {
        return len(p), nil
    }
    if len(p) == 0 && !rws.handlerDone {
        return 0, nil
    }

    if rws.handlerDone {
        rws.promoteUndeclaredTrailers()
    }

    // only send trailers if they have actually been defined by the
    // server handler.
    hasNonemptyTrailers := rws.hasNonemptyTrailers()
    endStream := rws.handlerDone && !hasNonemptyTrailers
    if len(p) > 0 || endStream {
        // only send a 0 byte DATA frame if we're ending the stream.
        if err := rws.conn.writeDataFromHandler(rws.stream, p, endStream); err != nil {
            rws.dirty = true
            return 0, err
        }
    }

    if rws.handlerDone && hasNonemptyTrailers {
        err = rws.conn.writeHeaders(rws.stream, &writeResHeaders{
            streamID:  rws.stream.id,
            h:         rws.handlerHeader,
            trailers:  rws.trailers,
            endStream: true,
        })
        if err != nil {
            rws.dirty = true
        }
        return len(p), err
    }
    return len(p), nil
}

[andybons]

@bradfitz @tombergan