code review 6260053: net/http: add new Server benchmark

net/http: add new Server benchmark

The new BenchmarkServer avoids profiling the client code
by running it in a child process.

[bradfitz, 2012-05-29 23:35:37]

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

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

[rsc, 2012-06-03 03:43:44]

http://codereview.appspot.com/6260053/diff/2001/src/pkg/net/http/serve_test.go
File src/pkg/net/http/serve_test.go (right):

http://codereview.appspot.com/6260053/diff/2001/src/pkg/net/http/serve_test.g...
src/pkg/net/http/serve_test.go:1252: ExportFormatTime(t)
Put these benchmarks in one of the many *_test.go files that is in package http,
and then you don't need the Export hack.

[bradfitz, 2012-06-03 19:52:38]

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

Please take another look.

[rsc, 2012-06-03 19:55:23]

Is the custom format still worth having at all?

[bradfitz, 2012-06-03 23:36:50]

On Sun, Jun 3, 2012 at 12:55 PM, Russ Cox <rsc@golang.org> wrote:

> Is the custom format still worth having at all?
>

Are further Time.Format speed-ups coming?  Even with your latest changes,
Format still shows up at 3.5%, which feels awkwardly high for an optional
response header that some servers don't include.

[rsc, 2012-06-03 23:53:13]

On Sun, Jun 3, 2012 at 7:36 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Are further Time.Format speed-ups coming?

Yes, eventually

> Even with your latest changes,
> Format still shows up at 3.5%, which feels awkwardly high for an optional
> response header that some servers don't include.

Is saving 2% worth custom code?

Russ

[bradfitz, 2012-06-04 00:23:04]

On Sun, Jun 3, 2012 at 4:52 PM, Russ Cox <rsc@golang.org> wrote:

> On Sun, Jun 3, 2012 at 7:36 PM, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> > Are further Time.Format speed-ups coming?
>
> Yes, eventually
>
> > Even with your latest changes,
> > Format still shows up at 3.5%, which feels awkwardly high for an optional
> > response header that some servers don't include.
>
> Is saving 2% worth custom code?
>

I was planning on doing 2+% in 10 places.

But I can see that might not fly.

Also, the custom code was already written at the time this saved more than
2%.  It doesn't affect the API.

I'll re-send this CL with just the benchmark part.

[bradfitz, 2012-06-04 00:24:07]

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

Please take another look.

[rsc, 2012-06-04 14:17:45]

On Sun, Jun 3, 2012 at 8:23 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> I was planning on doing 2+% in 10 places.
>
> But I can see that might not fly.
>
> Also, the custom code was already written at the time this saved more than
> 2%.  It doesn't affect the API.
>
> I'll re-send this CL with just the benchmark part.

Thanks. I don't mind 2% if it's fixing internal stuff, but 2% by
recreating the functionality already provided by the standard library
seems like overkill. The difference between yours and package time
right now is the latter's extra malloc for the byte buffer. Better
escape analysis will fix that without any changes to the code, so I'd
prefer to avoid installing the custom implementation. Just count on an
extra 2% at some point down the road, likely soon (I've done the first
half of the necessary changes already).

Russ

[bradfitz, 2012-06-04 14:32:48]

On Mon, Jun 4, 2012 at 7:17 AM, Russ Cox <rsc@golang.org> wrote:

> On Sun, Jun 3, 2012 at 8:23 PM, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> > I was planning on doing 2+% in 10 places.
> >
> > But I can see that might not fly.
> >
> > Also, the custom code was already written at the time this saved more
> than
> > 2%.  It doesn't affect the API.
> >
> > I'll re-send this CL with just the benchmark part.
>
> Thanks. I don't mind 2% if it's fixing internal stuff, but 2% by
> recreating the functionality already provided by the standard library
> seems like overkill. The difference between yours and package time
> right now is the latter's extra malloc for the byte buffer. Better
> escape analysis will fix that without any changes to the code, so I'd
> prefer to avoid installing the custom implementation. Just count on an
> extra 2% at some point down the road, likely soon (I've done the first
> half of the necessary changes already).
>

 Cool. Is this benchmark-only CL fine then?

[rsc, 2012-06-04 14:43:40]

LGTM

[bradfitz, 2012-06-04 15:06:08]

*** Submitted as http://code.google.com/p/go/source/detail?r=7d28afbc8273 ***

net/http: add new Server benchmark

The new BenchmarkServer avoids profiling the client code
by running it in a child process.

R=rsc
CC=golang-dev
http://codereview.appspot.com/6260053