code review 84230043: net/textproto: simplify common header interning

net/textproto: simplify common header interning

Takes advantage of CL 83740044, to optimize map[string] lookup
from []byte key.

Deletes code.

No conditional check for gccgo, since Ian plans to add this
to gccgo before GCC 4.10 (Go 1.3).

benchmark                   old ns/op     new ns/op     delta
BenchmarkReadMIMEHeader     6066          5086          -16.16%

benchmark                   old allocs     new allocs     delta
BenchmarkReadMIMEHeader     12             12             +0.00%

benchmark                   old bytes     new bytes     delta
BenchmarkReadMIMEHeader     1317          1317          +0.00%

Update Issue 3512

[bradfitz, 2014-04-03 21:36:16]

Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com, iant@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dave_cheney.net, 2014-04-03 23:42:16]

https://codereview.appspot.com/84230043/diff/60001/src/pkg/net/textproto/read...
File src/pkg/net/textproto/reader.go (right):

https://codereview.appspot.com/84230043/diff/60001/src/pkg/net/textproto/read...
src/pkg/net/textproto/reader.go:594: for _, v := range []string{
Is there any advantage to doing this in an init block vs

var commonHeader = map[string]string {
     "Accept": "Accept",
     ...
}

Does the compiler intern two string literals as the same read only data section
?

[bradfitz, 2014-04-03 23:46:03]

They're equivalent but my way looks lighter with less visual stutter.
 On Apr 3, 2014 4:42 PM, <dave@cheney.net> wrote:

>
> https://codereview.appspot.com/84230043/diff/60001/src/
> pkg/net/textproto/reader.go
> File src/pkg/net/textproto/reader.go (right):
>
> https://codereview.appspot.com/84230043/diff/60001/src/
> pkg/net/textproto/reader.go#newcode594
> src/pkg/net/textproto/reader.go:594: for _, v := range []string{
> Is there any advantage to doing this in an init block vs
>
> var commonHeader = map[string]string {
>      "Accept": "Accept",
>      ...
> }
>
> Does the compiler intern two string literals as the same read only data
> section ?
>
> https://codereview.appspot.com/84230043/
>

[rsc, 2014-04-07 13:54:43]

LGTM

https://codereview.appspot.com/84230043/diff/60001/src/pkg/net/textproto/read...
File src/pkg/net/textproto/reader.go (right):

https://codereview.appspot.com/84230043/diff/60001/src/pkg/net/textproto/read...
src/pkg/net/textproto/reader.go:594: for _, v := range []string{
On 2014/04/03 23:42:17, dfc wrote:
> Is there any advantage to doing this in an init block vs
> 
> var commonHeader = map[string]string {
>      "Accept": "Accept",
>      ...
> }
> 
> Does the compiler intern two string literals as the same read only data
section
> ?

The disadvantage to this is that the linker can possibly throw away an unused
variable if you use a real initializer. It cannot throw it away in this case.
But it's fine either way.

[bradfitz, 2014-04-07 17:39:01]

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

net/textproto: simplify common header interning

Takes advantage of CL 83740044, to optimize map[string] lookup
from []byte key.

Deletes code.

No conditional check for gccgo, since Ian plans to add this
to gccgo before GCC 4.10 (Go 1.3).

benchmark                   old ns/op     new ns/op     delta
BenchmarkReadMIMEHeader     6066          5086          -16.16%

benchmark                   old allocs     new allocs     delta
BenchmarkReadMIMEHeader     12             12             +0.00%

benchmark                   old bytes     new bytes     delta
BenchmarkReadMIMEHeader     1317          1317          +0.00%

Update Issue 3512

LGTM=rsc
R=rsc, dave
CC=golang-codereviews, iant
https://codereview.appspot.com/84230043

[gobot, 2014-04-07 17:51:39]

This CL appears to have broken the linux-386 builder.
See http://build.golang.org/log/eca3d6937d0772be43f854ad816837cb6571d30a

[bradfitz, 2014-04-07 17:53:31]

Looks unrelated but real.
 On Apr 7, 2014 10:51 AM, <gobot@golang.org> wrote:

> This CL appears to have broken the linux-386 builder.
> See http://build.golang.org/log/eca3d6937d0772be43f854ad816837cb6571d30a
>
> https://codereview.appspot.com/84230043/
>