code review 7300098: net/textproto: more efficient header parsing

net/textproto: more efficient header parsing

A co creation with bradfitz

* add fast path for header lines which are not continuations
* pass hint to better size initial mime header map

lucky(~/go/src/pkg/net/http) % ~/go/misc/benchcmp {golden,new}.txt
benchmark                          old ns/op    new ns/op    delta
BenchmarkReadRequestChrome             10073         8348  -17.12%
BenchmarkReadRequestCurl                4368         4350   -0.41%
BenchmarkReadRequestApachebench         4412         4397   -0.34%
BenchmarkReadRequestSiege               6431         5924   -7.88%
BenchmarkReadRequestWrk                 2820         3146  +11.56%

benchmark                           old MB/s     new MB/s  speedup
BenchmarkReadRequestChrome             60.66        73.18    1.21x
BenchmarkReadRequestCurl               17.85        17.93    1.00x
BenchmarkReadRequestApachebench        18.58        18.65    1.00x
BenchmarkReadRequestSiege              23.48        25.49    1.09x
BenchmarkReadRequestWrk                14.18        12.71    0.90x

benchmark                         old allocs   new allocs    delta
BenchmarkReadRequestChrome                32           26  -18.75%
BenchmarkReadRequestCurl                  15           15    0.00%
BenchmarkReadRequestApachebench           16           15   -6.25%
BenchmarkReadRequestSiege                 22           19  -13.64%
BenchmarkReadRequestWrk                   11           11    0.00%

benchmark                          old bytes    new bytes    delta
BenchmarkReadRequestChrome              3148         2216  -29.61%
BenchmarkReadRequestCurl                 905         1413   56.13%
BenchmarkReadRequestApachebench          956         1413   47.80%
BenchmarkReadRequestSiege               1397         1522    8.95%
BenchmarkReadRequestWrk                  757         1369   80.85%

[bradfitz, 2013-02-12 23:37:15]

https://codereview.appspot.com/7300098/diff/2001/src/pkg/net/textproto/textpr...
File src/pkg/net/textproto/textproto.go (right):

https://codereview.appspot.com/7300098/diff/2001/src/pkg/net/textproto/textpr...
src/pkg/net/textproto/textproto.go:152: b |= 0x20 // make upper case
lower case

[dave_cheney.net, 2013-02-13 04:57:49]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-02-13 05:27:06]

LGTM after nits

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/reader.go
File src/pkg/net/textproto/reader.go (right):

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/reader...
src/pkg/net/textproto/reader.go:131: // Optimistically assume we have buffered
data and we can
I know I wrote this sentence, but it's not really a sentence.  I think it's
missing a word or two.  Feel free to rewrite it all, since I can barely remember
what this does months later, despite writing the comment.

Maybe:

// Optimistically assume that we have a buffered line that
// starts with an ASCII letter (the next header key), so we can avoid
// copying that buffered data around in memory and skipping over
// non-existent whitespace.

Which is maybe a bit long, but at least says what's going on and isn't missing
words.

Alternatives welcomes, if you care.

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/textpr...
File src/pkg/net/textproto/textproto.go (right):

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/textpr...
src/pkg/net/textproto/textproto.go:153: return (b >= 'a' && b <= 'z')
drop parens.  also, I think Russ prefers the style:

return 'a' <= b && b <= 'z'

(so the bees are next to each other)

[dave_cheney.net, 2013-02-13 09:00:21]

Thanks Brad, PTAL. I'll leave this a while for others to comment.

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/reader.go
File src/pkg/net/textproto/reader.go (right):

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/reader...
src/pkg/net/textproto/reader.go:131: // Optimistically assume we have buffered
data and we can
Done. Yes, it took me a little time to understand what was happening hear.
RFC2822 folded lines are rare, especially in the HTTP context.

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/reader...
src/pkg/net/textproto/reader.go:131: // Optimistically assume we have buffered
data and we can
On 2013/02/13 05:27:06, bradfitz wrote:
> I know I wrote this sentence, but it's not really a sentence.  I think it's
> missing a word or two.  Feel free to rewrite it all, since I can barely
remember
> what this does months later, despite writing the comment.
> 
> Maybe:
> 
> // Optimistically assume that we have a buffered line that
> // starts with an ASCII letter (the next header key), so we can avoid
> // copying that buffered data around in memory and skipping over
> // non-existent whitespace.
> 
> Which is maybe a bit long, but at least says what's going on and isn't missing
> words.
> 
> Alternatives welcomes, if you care.
> 

Done.

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/textpr...
File src/pkg/net/textproto/textproto.go (right):

https://codereview.appspot.com/7300098/diff/5002/src/pkg/net/textproto/textpr...
src/pkg/net/textproto/textproto.go:153: return (b >= 'a' && b <= 'z')
On 2013/02/13 05:27:06, bradfitz wrote:
> drop parens.  also, I think Russ prefers the style:
> 
> return 'a' <= b && b <= 'z'
> 
> (so the bees are next to each other)

Done.

[dave_cheney.net, 2013-02-14 08:35:51]

*** Submitted as https://code.google.com/p/go/source/detail?r=a394d9eeb49a ***

net/textproto: more efficient header parsing

A co creation with bradfitz

* add fast path for header lines which are not continuations
* pass hint to better size initial mime header map

lucky(~/go/src/pkg/net/http) % ~/go/misc/benchcmp {golden,new}.txt
benchmark                          old ns/op    new ns/op    delta
BenchmarkReadRequestChrome             10073         8348  -17.12%
BenchmarkReadRequestCurl                4368         4350   -0.41%
BenchmarkReadRequestApachebench         4412         4397   -0.34%
BenchmarkReadRequestSiege               6431         5924   -7.88%
BenchmarkReadRequestWrk                 2820         3146  +11.56%

benchmark                           old MB/s     new MB/s  speedup
BenchmarkReadRequestChrome             60.66        73.18    1.21x
BenchmarkReadRequestCurl               17.85        17.93    1.00x
BenchmarkReadRequestApachebench        18.58        18.65    1.00x
BenchmarkReadRequestSiege              23.48        25.49    1.09x
BenchmarkReadRequestWrk                14.18        12.71    0.90x

benchmark                         old allocs   new allocs    delta
BenchmarkReadRequestChrome                32           26  -18.75%
BenchmarkReadRequestCurl                  15           15    0.00%
BenchmarkReadRequestApachebench           16           15   -6.25%
BenchmarkReadRequestSiege                 22           19  -13.64%
BenchmarkReadRequestWrk                   11           11    0.00%

benchmark                          old bytes    new bytes    delta
BenchmarkReadRequestChrome              3148         2216  -29.61%
BenchmarkReadRequestCurl                 905         1413   56.13%
BenchmarkReadRequestApachebench          956         1413   47.80%
BenchmarkReadRequestSiege               1397         1522    8.95%
BenchmarkReadRequestWrk                  757         1369   80.85%

R=bradfitz
CC=golang-dev
https://codereview.appspot.com/7300098