Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
heidawei opened this issue Mar 28, 2015 · 7 comments
Closed
Milestone

Comments

@heidawei
Copy link

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
Copy link
Contributor

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
Copy link
Author

@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.

@mikioh mikioh changed the title io.Copy() in function of http ReverseProxy is not high efficiency, I find a high efficiency way net/http/httputil: io.Copy in function of ReverseProxy is not high efficiency Mar 28, 2015
@heidawei
Copy link
Author

@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
Copy link
Contributor

rsc commented Apr 10, 2015

-> bradfitz to decide whether to close

@bradfitz
Copy link
Contributor

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
Copy link
Author

@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
Copy link

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

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jun 3, 2015
@golang golang locked and limited conversation to collaborators Nov 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants