From 68998433556a5c9ee0f7d8d544ffb006a724adaf Mon Sep 17 00:00:00 2001 From: Cyrill Schumacher Date: Sat, 20 Aug 2016 11:32:32 +0200 Subject: [PATCH] net/http: optimize internal cookie functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - precalculate *Cookie slice in read cookie functions - readSetCookies: pre-allocs depending on the count of Set-Cookies - rename success variable to ok; avoid else - refactor Cookie.String to use less allocations - remove fmt package and replace with writes to a bytes.Buffer - add BenchmarkReadSetCookies and BenchmarkReadCookies name old time/op new time/op delta CookieString-8 1.42µs ± 2% 0.78µs ± 1% -45.36% (p=0.000 n=10+10) ReadSetCookies-8 3.46µs ± 1% 3.42µs ± 2% -1.39% (p=0.001 n=10+10) ReadCookies-8 5.12µs ± 1% 5.15µs ± 2% ~ (p=0.393 n=10+10) name old alloc/op new alloc/op delta CookieString-8 520B ± 0% 384B ± 0% -26.15% (p=0.000 n=10+10) ReadSetCookies-8 968B ± 0% 960B ± 0% -0.83% (p=0.000 n=10+10) ReadCookies-8 2.01kB ± 0% 2.01kB ± 0% ~ (all samples are equal) name old allocs/op new allocs/op delta CookieString-8 10.0 ± 0% 3.0 ± 0% -70.00% (p=0.000 n=10+10) ReadSetCookies-8 18.0 ± 0% 17.0 ± 0% -5.56% (p=0.000 n=10+10) ReadCookies-8 16.0 ± 0% 16.0 ± 0% ~ (all samples are equal) Change-Id: I870670987f10f3e52f9c657cfb8e6eaaa97a6162 Reviewed-on: https://go-review.googlesource.com/27850 Run-TryBot: Russ Cox Reviewed-by: Russ Cox --- src/net/http/cookie.go | 58 ++++++++++++++---------- src/net/http/cookie_test.go | 89 +++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 23 deletions(-) diff --git a/src/net/http/cookie.go b/src/net/http/cookie.go index 1ea0e9397a..a0a4690ddc 100644 --- a/src/net/http/cookie.go +++ b/src/net/http/cookie.go @@ -6,7 +6,6 @@ package http import ( "bytes" - "fmt" "log" "net" "strconv" @@ -40,7 +39,11 @@ type Cookie struct { // readSetCookies parses all "Set-Cookie" values from // the header h and returns the successfully parsed Cookies. func readSetCookies(h Header) []*Cookie { - cookies := []*Cookie{} + cookieCount := len(h["Set-Cookie"]) + if cookieCount == 0 { + return []*Cookie{} + } + cookies := make([]*Cookie, 0, cookieCount) for _, line := range h["Set-Cookie"] { parts := strings.Split(strings.TrimSpace(line), ";") if len(parts) == 1 && parts[0] == "" { @@ -55,8 +58,8 @@ func readSetCookies(h Header) []*Cookie { if !isCookieNameValid(name) { continue } - value, success := parseCookieValue(value, true) - if !success { + value, ok := parseCookieValue(value, true) + if !ok { continue } c := &Cookie{ @@ -75,8 +78,8 @@ func readSetCookies(h Header) []*Cookie { attr, val = attr[:j], attr[j+1:] } lowerAttr := strings.ToLower(attr) - val, success = parseCookieValue(val, false) - if !success { + val, ok = parseCookieValue(val, false) + if !ok { c.Unparsed = append(c.Unparsed, parts[i]) continue } @@ -96,10 +99,9 @@ func readSetCookies(h Header) []*Cookie { break } if secs <= 0 { - c.MaxAge = -1 - } else { - c.MaxAge = secs + secs = -1 } + c.MaxAge = secs continue case "expires": c.RawExpires = val @@ -142,9 +144,13 @@ func (c *Cookie) String() string { return "" } var b bytes.Buffer - fmt.Fprintf(&b, "%s=%s", sanitizeCookieName(c.Name), sanitizeCookieValue(c.Value)) + b.WriteString(sanitizeCookieName(c.Name)) + b.WriteRune('=') + b.WriteString(sanitizeCookieValue(c.Value)) + if len(c.Path) > 0 { - fmt.Fprintf(&b, "; Path=%s", sanitizeCookiePath(c.Path)) + b.WriteString("; Path=") + b.WriteString(sanitizeCookiePath(c.Path)) } if len(c.Domain) > 0 { if validCookieDomain(c.Domain) { @@ -156,25 +162,31 @@ func (c *Cookie) String() string { if d[0] == '.' { d = d[1:] } - fmt.Fprintf(&b, "; Domain=%s", d) + b.WriteString("; Domain=") + b.WriteString(d) } else { - log.Printf("net/http: invalid Cookie.Domain %q; dropping domain attribute", - c.Domain) + log.Printf("net/http: invalid Cookie.Domain %q; dropping domain attribute", c.Domain) } } if c.Expires.Unix() > 0 { - fmt.Fprintf(&b, "; Expires=%s", c.Expires.UTC().Format(TimeFormat)) + b.WriteString("; Expires=") + b2 := b.Bytes() + b.Reset() + b.Write(c.Expires.UTC().AppendFormat(b2, TimeFormat)) } if c.MaxAge > 0 { - fmt.Fprintf(&b, "; Max-Age=%d", c.MaxAge) + b.WriteString("; Max-Age=") + b2 := b.Bytes() + b.Reset() + b.Write(strconv.AppendInt(b2, int64(c.MaxAge), 10)) } else if c.MaxAge < 0 { - fmt.Fprintf(&b, "; Max-Age=0") + b.WriteString("; Max-Age=0") } if c.HttpOnly { - fmt.Fprintf(&b, "; HttpOnly") + b.WriteString("; HttpOnly") } if c.Secure { - fmt.Fprintf(&b, "; Secure") + b.WriteString("; Secure") } return b.String() } @@ -184,12 +196,12 @@ func (c *Cookie) String() string { // // if filter isn't empty, only cookies of that name are returned func readCookies(h Header, filter string) []*Cookie { - cookies := []*Cookie{} lines, ok := h["Cookie"] if !ok { - return cookies + return []*Cookie{} } + cookies := []*Cookie{} for _, line := range lines { parts := strings.Split(strings.TrimSpace(line), ";") if len(parts) == 1 && parts[0] == "" { @@ -212,8 +224,8 @@ func readCookies(h Header, filter string) []*Cookie { if filter != "" && filter != name { continue } - val, success := parseCookieValue(val, true) - if !success { + val, ok := parseCookieValue(val, true) + if !ok { continue } cookies = append(cookies, &Cookie{Name: name, Value: val}) diff --git a/src/net/http/cookie_test.go b/src/net/http/cookie_test.go index 95e61479a1..2c01040281 100644 --- a/src/net/http/cookie_test.go +++ b/src/net/http/cookie_test.go @@ -426,3 +426,92 @@ func TestCookieSanitizePath(t *testing.T) { t.Errorf("Expected substring %q in log output. Got:\n%s", sub, got) } } + +func BenchmarkCookieString(b *testing.B) { + const wantCookieString = `cookie-9=i3e01nf61b6t23bvfmplnanol3; Path=/restricted/; Domain=example.com; Expires=Tue, 10 Nov 2009 23:00:00 GMT; Max-Age=3600` + c := &Cookie{ + Name: "cookie-9", + Value: "i3e01nf61b6t23bvfmplnanol3", + Expires: time.Unix(1257894000, 0), + Path: "/restricted/", + Domain: ".example.com", + MaxAge: 3600, + } + var benchmarkCookieString string + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + benchmarkCookieString = c.String() + } + if have, want := benchmarkCookieString, wantCookieString; have != want { + b.Fatalf("Have: %v Want: %v", have, want) + } +} + +func BenchmarkReadSetCookies(b *testing.B) { + header := Header{ + "Set-Cookie": { + "NID=99=YsDT5i3E-CXax-; expires=Wed, 23-Nov-2011 01:05:03 GMT; path=/; domain=.google.ch; HttpOnly", + ".ASPXAUTH=7E3AA; expires=Wed, 07-Mar-2012 14:25:06 GMT; path=/; HttpOnly", + }, + } + wantCookies := []*Cookie{ + { + Name: "NID", + Value: "99=YsDT5i3E-CXax-", + Path: "/", + Domain: ".google.ch", + HttpOnly: true, + Expires: time.Date(2011, 11, 23, 1, 5, 3, 0, time.UTC), + RawExpires: "Wed, 23-Nov-2011 01:05:03 GMT", + Raw: "NID=99=YsDT5i3E-CXax-; expires=Wed, 23-Nov-2011 01:05:03 GMT; path=/; domain=.google.ch; HttpOnly", + }, + { + Name: ".ASPXAUTH", + Value: "7E3AA", + Path: "/", + Expires: time.Date(2012, 3, 7, 14, 25, 6, 0, time.UTC), + RawExpires: "Wed, 07-Mar-2012 14:25:06 GMT", + HttpOnly: true, + Raw: ".ASPXAUTH=7E3AA; expires=Wed, 07-Mar-2012 14:25:06 GMT; path=/; HttpOnly", + }, + } + var c []*Cookie + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + c = readSetCookies(header) + } + if !reflect.DeepEqual(c, wantCookies) { + b.Fatalf("readSetCookies:\nhave: %s\nwant: %s\n", toJSON(c), toJSON(wantCookies)) + } +} + +func BenchmarkReadCookies(b *testing.B) { + header := Header{ + "Cookie": { + `de=; client_region=0; rpld1=0:hispeed.ch|20:che|21:zh|22:zurich|23:47.36|24:8.53|; rpld0=1:08|; backplane-channel=newspaper.com:1471; devicetype=0; osfam=0; rplmct=2; s_pers=%20s_vmonthnum%3D1472680800496%2526vn%253D1%7C1472680800496%3B%20s_nr%3D1471686767664-New%7C1474278767664%3B%20s_lv%3D1471686767669%7C1566294767669%3B%20s_lv_s%3DFirst%2520Visit%7C1471688567669%3B%20s_monthinvisit%3Dtrue%7C1471688567677%3B%20gvp_p5%3Dsports%253Ablog%253Aearly-lead%2520-%2520184693%2520-%252020160820%2520-%2520u-s%7C1471688567681%3B%20gvp_p51%3Dwp%2520-%2520sports%7C1471688567684%3B; s_sess=%20s_wp_ep%3Dhomepage%3B%20s._ref%3Dhttps%253A%252F%252Fwww.google.ch%252F%3B%20s_cc%3Dtrue%3B%20s_ppvl%3Dsports%25253Ablog%25253Aearly-lead%252520-%252520184693%252520-%25252020160820%252520-%252520u-lawyer%252C12%252C12%252C502%252C1231%252C502%252C1680%252C1050%252C2%252CP%3B%20s_ppv%3Dsports%25253Ablog%25253Aearly-lead%252520-%252520184693%252520-%25252020160820%252520-%252520u-s-lawyer%252C12%252C12%252C502%252C1231%252C502%252C1680%252C1050%252C2%252CP%3B%20s_dslv%3DFirst%2520Visit%3B%20s_sq%3Dwpninewspapercom%253D%252526pid%25253Dsports%2525253Ablog%2525253Aearly-lead%25252520-%25252520184693%25252520-%2525252020160820%25252520-%25252520u-s%252526pidt%25253D1%252526oid%25253Dhttps%2525253A%2525252F%2525252Fwww.newspaper.com%2525252F%2525253Fnid%2525253Dmenu_nav_homepage%252526ot%25253DA%3B`, + }, + } + wantCookies := []*Cookie{ + {Name: "de", Value: ""}, + {Name: "client_region", Value: "0"}, + {Name: "rpld1", Value: "0:hispeed.ch|20:che|21:zh|22:zurich|23:47.36|24:8.53|"}, + {Name: "rpld0", Value: "1:08|"}, + {Name: "backplane-channel", Value: "newspaper.com:1471"}, + {Name: "devicetype", Value: "0"}, + {Name: "osfam", Value: "0"}, + {Name: "rplmct", Value: "2"}, + {Name: "s_pers", Value: "%20s_vmonthnum%3D1472680800496%2526vn%253D1%7C1472680800496%3B%20s_nr%3D1471686767664-New%7C1474278767664%3B%20s_lv%3D1471686767669%7C1566294767669%3B%20s_lv_s%3DFirst%2520Visit%7C1471688567669%3B%20s_monthinvisit%3Dtrue%7C1471688567677%3B%20gvp_p5%3Dsports%253Ablog%253Aearly-lead%2520-%2520184693%2520-%252020160820%2520-%2520u-s%7C1471688567681%3B%20gvp_p51%3Dwp%2520-%2520sports%7C1471688567684%3B"}, + {Name: "s_sess", Value: "%20s_wp_ep%3Dhomepage%3B%20s._ref%3Dhttps%253A%252F%252Fwww.google.ch%252F%3B%20s_cc%3Dtrue%3B%20s_ppvl%3Dsports%25253Ablog%25253Aearly-lead%252520-%252520184693%252520-%25252020160820%252520-%252520u-lawyer%252C12%252C12%252C502%252C1231%252C502%252C1680%252C1050%252C2%252CP%3B%20s_ppv%3Dsports%25253Ablog%25253Aearly-lead%252520-%252520184693%252520-%25252020160820%252520-%252520u-s-lawyer%252C12%252C12%252C502%252C1231%252C502%252C1680%252C1050%252C2%252CP%3B%20s_dslv%3DFirst%2520Visit%3B%20s_sq%3Dwpninewspapercom%253D%252526pid%25253Dsports%2525253Ablog%2525253Aearly-lead%25252520-%25252520184693%25252520-%2525252020160820%25252520-%25252520u-s%252526pidt%25253D1%252526oid%25253Dhttps%2525253A%2525252F%2525252Fwww.newspaper.com%2525252F%2525253Fnid%2525253Dmenu_nav_homepage%252526ot%25253DA%3B"}, + } + var c []*Cookie + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + c = readCookies(header, "") + } + if !reflect.DeepEqual(c, wantCookies) { + b.Fatalf("readCookies:\nhave: %s\nwant: %s\n", toJSON(c), toJSON(wantCookies)) + } +}