net/http/httputil: io.Copy in function of ReverseProxy is not high efficiency

[heidawei]

in funtion

func (p *ReverseProxy) copyResponse(dst io.Writer, src io.Reader) {
    if p.FlushInterval != 0 {
        if wf, ok := dst.(writeFlusher); ok {
            mlw := &maxLatencyWriter{
                dst:     wf,
                latency: p.FlushInterval,
                done:    make(chan bool),
            }
            go mlw.flushLoop()
            defer mlw.stop()
            dst = mlw
        }
    }

    io.Copy(dst, src)
}

in boom test the QPS is 160000, but use another way like this:

func (p *ReverseProxy) copyResponse(dst io.Writer, src io.Reader) {
    if p.FlushInterval != 0 {
        if wf, ok := dst.(writeFlusher); ok {
            mlw := &maxLatencyWriter{
                dst:     wf,
                latency: p.FlushInterval,
                done:    make(chan bool),
            }
            go mlw.flushLoop()
            defer mlw.stop()
            dst = mlw
        }
    }
        //now i Ignore the err temporary
    payload, err := ioutil.ReadAll(src)
    n, err := dst.Write(payload)
}

in boom test the QPS is 190000.

[randall77]

The problem with your code is that it has to read all the data into memory before writing it out. That's not something we should do unconditionally. To make it safe you'd have to find a way to do it only when the reader size is limited. Wrap in a LimitedReader, perhaps?

I don't understand why your code is faster. Perhaps io.Copy could be improved. It allocates 32KB chunks, maybe that's not the right size for your test case. (too large? too small? not sure.) More info about your queries would help.

[heidawei]

@randall77 there is only 50 or 128 or 1024 bytes in my http request's body. in my http service, the http handle is like this:

func handlePayloadCopy(w http.ResponseWriter, r *http.Request) {
    io.Copy(w, r.Body)
   defer r.Body.Close()
   w.Header().Set("Content-Length", fmt.Sprintf("%d", r.ContentLength))
}

func handlePayloadReadWrite(w http.ResponseWriter, r *http.Request) {
   payload, _ := ioutil.ReadAll(r.Body)
   w.Write(payload)
   defer r.Body.Close()
   w.Header().Set("Content-Length", fmt.Sprintf("%d", r.ContentLength))
}

Don't consider the safe, I just want to show which way is better. I do not modify the http ReverseProxy code, and I find in

ServeHTTP(w http.ResponseWriter, req *http.Request)

the problem is the same, so I write the code to prove.

[heidawei]

@randall77 the go version is 1.4, and I use LimitedReader, the result is the same with the Reader. and I see when I use (io.Copy or io.CopyN), the server's cpu is more than 500%(12 cpus), the QPS is 24000, but when I use (ReadAll + Write), the server's cpu is only 300%, and the QPS is 28000. I my really server, I see the same phenomenon. in my ServeHTTP, I replace the io.Copy to ReadAll+Write, the effect is very good. but I my web router server which use ReverseProxy, the code is fficial code, i do not modify it, but it cost more cpus.

[rsc]

-> bradfitz to decide whether to close

[bradfitz]

Perhaps we can add an optional hook to ReverseProxy to get buffers to use for copying, and then use the new io.CopyBuffer in https://go-review.googlesource.com/#/c/8737/ with those buffers

[heidawei]

@bradfitz, now I see when I test in Ali cloud host or Microsoft azure, the Performance of io.Copy is the same with the (ReadAll + Write), but in baidu cloud host, the Performance of io.Copy is bad, so maybe it is the system problem, not a code problem, i do not know

[gopherbot]

CL https://golang.org/cl/9399 mentions this issue.