net/http: use SplitSeq in ParseCookie

goos: linux
goarch: amd64
pkg: net/http
cpu: 12th Gen Intel(R) Core(TM) i7-12700
                           │ master.txt  │             patch.txt              │
                           │   sec/op    │   sec/op     vs base               │
ReadCookies/readCookies-20   1.499µ ± 1%   1.482µ ± 3%       ~ (p=0.197 n=10)
ReadCookies/ParseCookie-20   1.552µ ± 2%   1.437µ ± 2%  -7.44% (p=0.000 n=10)
geomean                      1.525µ        1.459µ       -4.34%

                           │  master.txt  │               patch.txt               │
                           │     B/op     │     B/op      vs base                 │
ReadCookies/readCookies-20   1.953Ki ± 0%   1.953Ki ± 0%       ~ (p=1.000 n=10) ¹
ReadCookies/ParseCookie-20   2.109Ki ± 0%   1.953Ki ± 0%  -7.41% (p=0.000 n=10)
geomean                      2.030Ki        1.953Ki       -3.77%
¹ all samples are equal

                           │ master.txt │              patch.txt              │
                           │ allocs/op  │ allocs/op   vs base                 │
ReadCookies/readCookies-20   11.00 ± 0%   11.00 ± 0%       ~ (p=1.000 n=10) ¹
ReadCookies/ParseCookie-20   12.00 ± 0%   11.00 ± 0%  -8.33% (p=0.000 n=10)
geomean                      11.49        11.00       -4.26%
¹ all samples are equal

Change-Id: I7e84d52ac8d25991595d1b0480334fe9d1a84ffb
Reviewed-on: https://go-review.googlesource.com/c/go/+/727520
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/src/net/http/cookie.go b/src/net/http/cookie.go
index 4cc9ae31522aeb98dfbfda5f702ca92c4aa4a07a..37cda60845051814a4f3b2ed5f382a598b20d536 100644 (file)
--- a/src/net/http/cookie.go
+++ b/src/net/http/cookie.go
@@ -89,15 +89,14 @@ func cookieNumWithinMax(cookieNum int) bool {
 // which were set in it. Since the same cookie name can appear multiple times
 // the returned Values can contain more than one value for a given key.
 func ParseCookie(line string) ([]*Cookie, error) {
-       if !cookieNumWithinMax(strings.Count(line, ";") + 1) {
+       nparts := strings.Count(line, ";") + 1
+       if !cookieNumWithinMax(nparts) {
                return nil, errCookieNumLimitExceeded
-       }
-       parts := strings.Split(textproto.TrimString(line), ";")
-       if len(parts) == 1 && parts[0] == "" {
+       } else if nparts == 1 && textproto.TrimString(line) == "" {
                return nil, errBlankCookie
        }
-       cookies := make([]*Cookie, 0, len(parts))
-       for _, s := range parts {
+       cookies := make([]*Cookie, 0, nparts)
+       for s := range strings.SplitSeq(line, ";") {
                s = textproto.TrimString(s)
                name, value, found := strings.Cut(s, "=")
                if !found {

diff --git a/src/net/http/cookie_test.go b/src/net/http/cookie_test.go
index f452b4ec76830f9b994a668f1d0a09917903e863..32c4361c401f25a33fd8d124e2c69562bb74d3f7 100644 (file)
--- a/src/net/http/cookie_test.go
+++ b/src/net/http/cookie_test.go
@@ -702,11 +702,8 @@ func BenchmarkReadSetCookies(b *testing.B) {
 }
 
 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`,
-               },
-       }
+       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`
+       header := Header{"Cookie": {cookie}}
        wantCookies := []*Cookie{
                {Name: "de", Value: ""},
                {Name: "client_region", Value: "0"},
@@ -720,14 +717,26 @@ func BenchmarkReadCookies(b *testing.B) {
                {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, "")
-       }
+
+       b.Run("readCookies", func(b *testing.B) {
+               b.ReportAllocs()
+               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))
        }
+
+       b.Run("ParseCookie", func(b *testing.B) {
+               b.ReportAllocs()
+               for i := 0; i < b.N; i++ {
+                       c, _ = ParseCookie(cookie)
+               }
+       })
+       if !reflect.DeepEqual(c, wantCookies) {
+               b.Fatalf("ParseCookie:\nhave: %s\nwant: %s\n", toJSON(c), toJSON(wantCookies))
+       }
 }
 
 func TestParseCookie(t *testing.T) {