From beae16f07c78302ca8d41dcc8665107101d20187 Mon Sep 17 00:00:00 2001 From: Tw Date: Thu, 21 Jul 2016 01:06:14 +0000 Subject: [PATCH] Proxy performance (#946) * proxy: add benchmark Signed-off-by: Tw * replacer: prepare lazily update issue#939 benchmark old ns/op new ns/op delta BenchmarkProxy-4 83865 72544 -13.50% Signed-off-by: Tw * proxy: use buffer pool to avoid temporary allocation Signed-off-by: Tw --- caddyhttp/httpserver/replacer.go | 68 +++++++++++++++------------ caddyhttp/httpserver/replacer_test.go | 23 ++++++--- caddyhttp/proxy/proxy_test.go | 34 ++++++++++++++ caddyhttp/proxy/reverseproxy.go | 11 ++++- 4 files changed, 99 insertions(+), 37 deletions(-) diff --git a/caddyhttp/httpserver/replacer.go b/caddyhttp/httpserver/replacer.go index dea2f776..41c68114 100644 --- a/caddyhttp/httpserver/replacer.go +++ b/caddyhttp/httpserver/replacer.go @@ -37,8 +37,8 @@ type Replacer interface { // they will be used to overwrite other replacements // if there is a name conflict. type replacer struct { - replacements map[string]string - customReplacements map[string]string + replacements map[string]func() string + customReplacements map[string]func() string emptyValue string responseRecorder *ResponseRecorder } @@ -53,36 +53,36 @@ type replacer struct { func NewReplacer(r *http.Request, rr *ResponseRecorder, emptyValue string) Replacer { rep := &replacer{ responseRecorder: rr, - customReplacements: make(map[string]string), - replacements: map[string]string{ - "{method}": r.Method, + customReplacements: make(map[string]func() string), + replacements: map[string]func() string{ + "{method}": func() string { return r.Method }, "{scheme}": func() string { if r.TLS != nil { return "https" } return "http" - }(), + }, "{hostname}": func() string { name, err := os.Hostname() if err != nil { return "" } return name - }(), - "{host}": r.Host, + }, + "{host}": func() string { return r.Host }, "{hostonly}": func() string { host, _, err := net.SplitHostPort(r.Host) if err != nil { return r.Host } return host - }(), - "{path}": r.URL.Path, - "{path_escaped}": url.QueryEscape(r.URL.Path), - "{query}": r.URL.RawQuery, - "{query_escaped}": url.QueryEscape(r.URL.RawQuery), - "{fragment}": r.URL.Fragment, - "{proto}": r.Proto, + }, + "{path}": func() string { return r.URL.Path }, + "{path_escaped}": func() string { return url.QueryEscape(r.URL.Path) }, + "{query}": func() string { return r.URL.RawQuery }, + "{query_escaped}": func() string { return url.QueryEscape(r.URL.RawQuery) }, + "{fragment}": func() string { return r.URL.Fragment }, + "{proto}": func() string { return r.Proto }, "{remote}": func() string { if fwdFor := r.Header.Get("X-Forwarded-For"); fwdFor != "" { return fwdFor @@ -92,25 +92,25 @@ func NewReplacer(r *http.Request, rr *ResponseRecorder, emptyValue string) Repla return r.RemoteAddr } return host - }(), + }, "{port}": func() string { _, port, err := net.SplitHostPort(r.RemoteAddr) if err != nil { return "" } return port - }(), - "{uri}": r.URL.RequestURI(), - "{uri_escaped}": url.QueryEscape(r.URL.RequestURI()), - "{when}": time.Now().Format(timeFormat), + }, + "{uri}": func() string { return r.URL.RequestURI() }, + "{uri_escaped}": func() string { return url.QueryEscape(r.URL.RequestURI()) }, + "{when}": func() string { return time.Now().Format(timeFormat) }, "{file}": func() string { _, file := path.Split(r.URL.Path) return file - }(), + }, "{dir}": func() string { dir, _ := path.Split(r.URL.Path) return dir - }(), + }, "{request}": func() string { dump, err := httputil.DumpRequest(r, false) if err != nil { @@ -118,14 +118,15 @@ func NewReplacer(r *http.Request, rr *ResponseRecorder, emptyValue string) Repla } return requestReplacer.Replace(string(dump)) - }(), + }, }, emptyValue: emptyValue, } // Header placeholders (case-insensitive) for header, values := range r.Header { - rep.replacements[headerReplacer+strings.ToLower(header)+"}"] = strings.Join(values, ",") + values := values + rep.replacements[headerReplacer+strings.ToLower(header)+"}"] = func() string { return strings.Join(values, ",") } } return rep @@ -141,9 +142,9 @@ func (r *replacer) Replace(s string) string { // Make response placeholders now if r.responseRecorder != nil { - r.replacements["{status}"] = strconv.Itoa(r.responseRecorder.status) - r.replacements["{size}"] = strconv.Itoa(r.responseRecorder.size) - r.replacements["{latency}"] = time.Since(r.responseRecorder.start).String() + r.replacements["{status}"] = func() string { return strconv.Itoa(r.responseRecorder.status) } + r.replacements["{size}"] = func() string { return strconv.Itoa(r.responseRecorder.size) } + r.replacements["{latency}"] = func() string { return time.Since(r.responseRecorder.start).String() } } // Include custom placeholders, overwriting existing ones if necessary @@ -158,7 +159,10 @@ func (r *replacer) Replace(s string) string { idxEnd := strings.Index(s[endOffset:], "}") if idxEnd > -1 { placeholder := strings.ToLower(s[idxStart : endOffset+idxEnd+1]) - replacement := r.replacements[placeholder] + replacement := "" + if getReplacement, ok := r.replacements[placeholder]; ok { + replacement = getReplacement() + } if replacement == "" { replacement = r.emptyValue } @@ -169,7 +173,11 @@ func (r *replacer) Replace(s string) string { } // Regular replacements - these are easier because they're case-sensitive - for placeholder, replacement := range r.replacements { + for placeholder, getReplacement := range r.replacements { + if !strings.Contains(s, placeholder) { + continue + } + replacement := getReplacement() if replacement == "" { replacement = r.emptyValue } @@ -181,7 +189,7 @@ func (r *replacer) Replace(s string) string { // Set sets key to value in the r.customReplacements map. func (r *replacer) Set(key, value string) { - r.customReplacements["{"+key+"}"] = value + r.customReplacements["{"+key+"}"] = func() string { return value } } const ( diff --git a/caddyhttp/httpserver/replacer_test.go b/caddyhttp/httpserver/replacer_test.go index 5768c42f..cb72ba5e 100644 --- a/caddyhttp/httpserver/replacer_test.go +++ b/caddyhttp/httpserver/replacer_test.go @@ -21,19 +21,26 @@ func TestNewReplacer(t *testing.T) { switch v := rep.(type) { case *replacer: - if v.replacements["{host}"] != "localhost" { + if v.replacements["{host}"]() != "localhost" { t.Error("Expected host to be localhost") } - if v.replacements["{method}"] != "POST" { + if v.replacements["{method}"]() != "POST" { t.Error("Expected request method to be POST") } // Response placeholders should only be set after call to Replace() - if got, want := v.replacements["{status}"], ""; got != want { + got, want := "", "" + if getReplacement, ok := v.replacements["{status}"]; ok { + got = getReplacement() + } + if want := ""; got != want { t.Errorf("Expected status to NOT be set before Replace() is called; was: %s", got) } rep.Replace("{foobar}") - if got, want := v.replacements["{status}"], "200"; got != want { + if getReplacement, ok := v.replacements["{status}"]; ok { + got = getReplacement() + } + if want = "200"; got != want { t.Errorf("Expected status to be %s, was: %s", want, got) } default: @@ -84,10 +91,14 @@ func TestReplace(t *testing.T) { complexCases := []struct { template string - replacements map[string]string + replacements map[string]func() string expect string }{ - {"/a{1}/{2}", map[string]string{"{1}": "12", "{2}": ""}, "/a12/"}, + {"/a{1}/{2}", + map[string]func() string{ + "{1}": func() string { return "12" }, + "{2}": func() string { return "" }}, + "/a12/"}, } for _, c := range complexCases { diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index e13a3d46..ae0ca0f1 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -761,3 +761,37 @@ func (c *fakeConn) SetWriteDeadline(t time.Time) error { return nil } func (c *fakeConn) Close() error { return nil } func (c *fakeConn) Read(b []byte) (int, error) { return c.readBuf.Read(b) } func (c *fakeConn) Write(b []byte) (int, error) { return c.writeBuf.Write(b) } + +func BenchmarkProxy(b *testing.B) { + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("Hello, client")) + })) + defer backend.Close() + + upstream := newFakeUpstream(backend.URL, false) + upstream.host.UpstreamHeaders = http.Header{ + "Hostname": {"{hostname}"}, + "Host": {"{host}"}, + "X-Real-IP": {"{remote}"}, + "X-Forwarded-Proto": {"{scheme}"}, + } + // set up proxy + p := &Proxy{ + Next: httpserver.EmptyNext, // prevents panic in some cases when test fails + Upstreams: []Upstream{upstream}, + } + + w := httptest.NewRecorder() + + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + // create request and response recorder + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + b.Fatalf("Failed to create request: %v", err) + } + b.StartTimer() + p.ServeHTTP(w, r) + } +} diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go index 5a14aea7..c92746dc 100644 --- a/caddyhttp/proxy/reverseproxy.go +++ b/caddyhttp/proxy/reverseproxy.go @@ -22,6 +22,12 @@ import ( "time" ) +var bufferPool = sync.Pool{New: createBuffer} + +func createBuffer() interface{} { + return make([]byte, 32*1024) +} + // onExitFlushLoop is a callback set by tests to detect the state of the // flushLoop() goroutine. var onExitFlushLoop func() @@ -214,6 +220,9 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, outreq *http.Request, r } func (p *ReverseProxy) copyResponse(dst io.Writer, src io.Reader) { + buf := bufferPool.Get() + defer bufferPool.Put(buf) + if p.FlushInterval != 0 { if wf, ok := dst.(writeFlusher); ok { mlw := &maxLatencyWriter{ @@ -226,7 +235,7 @@ func (p *ReverseProxy) copyResponse(dst io.Writer, src io.Reader) { dst = mlw } } - io.Copy(dst, src) + io.CopyBuffer(dst, src, buf.([]byte)) } type writeFlusher interface {