]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: optimize internal cookie functions
authorCyrill Schumacher <cyrill@schumacher.fm>
Sat, 20 Aug 2016 09:32:32 +0000 (11:32 +0200)
committerRuss Cox <rsc@golang.org>
Wed, 12 Oct 2016 14:59:36 +0000 (14:59 +0000)
- 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 <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
src/net/http/cookie.go
src/net/http/cookie_test.go

index 1ea0e9397a37ec41042931a3a480d081334ff470..a0a4690ddc195fa9fae8ee1ef221c1f7b28b3062 100644 (file)
@@ -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})
index 95e61479a1599044fef0fcf92c18fd87b5cd9162..2c01040281bc95ed972ed7cccabf1b8750e4a52a 100644 (file)
@@ -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))
+       }
+}