code review 8264043: net: enable runtime-integrated network pollster on free...

net: enable runtime-integrated network pollster on freebsd, openbsd

Fixes issue 5199.

Benchmark results on freebsd/amd64 (virtual machine):

benchmark                             old ns/op    new ns/op    delta
BenchmarkTCP4OneShot-2                   184566       187164   +1.41%
BenchmarkTCP4OneShotTimeout-2            215558       187722  -12.91%
BenchmarkTCP4Persistent-2                 59686        41294  -30.81%
BenchmarkTCP4PersistentTimeout-2          60692        39974  -34.14%
BenchmarkTCP6OneShot-2                   226595       223688   -1.28%
BenchmarkTCP6OneShotTimeout-2            253144       225161  -11.05%
BenchmarkTCP6Persistent-2                 69157        55605  -19.60%
BenchmarkTCP6PersistentTimeout-2          70426        53805  -23.60%
BenchmarkTCP4ConcurrentReadWrite-2        53878        56087   +4.10%
BenchmarkTCP6ConcurrentReadWrite-2        66538        68190   +2.48%

benchmark                            old allocs   new allocs    delta
BenchmarkTCP4OneShot-2                       39           36   -7.69%
BenchmarkTCP4OneShotTimeout-2                42           36  -14.29%
BenchmarkTCP4Persistent-2                     1            0  -100.00%
BenchmarkTCP4PersistentTimeout-2              1            0  -100.00%
BenchmarkTCP6OneShot-2                       41           36  -12.20%
BenchmarkTCP6OneShotTimeout-2                43           36  -16.28%
BenchmarkTCP6Persistent-2                     1            0  -100.00%
BenchmarkTCP6PersistentTimeout-2              1            0  -100.00%
BenchmarkTCP4ConcurrentReadWrite-2            0            0     n/a%
BenchmarkTCP6ConcurrentReadWrite-2            0            0     n/a%

benchmark                             old bytes    new bytes    delta
BenchmarkTCP4OneShot-2                     3084         2544  -17.51%
BenchmarkTCP4OneShotTimeout-2              3129         2519  -19.50%
BenchmarkTCP4Persistent-2                    30            0  -100.00%
BenchmarkTCP4PersistentTimeout-2             31            0  -100.00%
BenchmarkTCP6OneShot-2                     3297         2660  -19.32%
BenchmarkTCP6OneShotTimeout-2              3306         2655  -19.69%
BenchmarkTCP6Persistent-2                    31            0  -100.00%
BenchmarkTCP6PersistentTimeout-2             29            0  -100.00%
BenchmarkTCP4ConcurrentReadWrite-2            2            0  -100.00%
BenchmarkTCP6ConcurrentReadWrite-2            7            0  -100.00%

[mikio, 2013-04-03 05:27:38]

Hello dvyukov@google.com, minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

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

[mikio, 2013-04-03 05:47:38]

not so rush, of course.

[dave_cheney.net, 2013-04-03 05:52:15]

> not so rush, of course.
>
> https://codereview.appspot.com/8264043/


Hello. Are you happy for this to sit in the queue until after 1.1 ? I
guess the folks that are really keen can apply your patch from this
CL.

Cheers

Dave

[mikio, 2013-04-03 06:25:18]

> Hello. Are you happy for this to sit in the queue until after 1.1 ? I
> guess the folks that are really keen can apply your patch from this
> CL.

i'm fine either, this CL does nothing to go 1.1 api.
so cl 6445105 is more important to me.

[bradfitz, 2013-04-03 07:40:53]

Mikioh,

Please file an issue for this, though, so it's not forgotten, and
cross-link this CL and the issue.




On Tue, Apr 2, 2013 at 10:47 PM, <mikioh.mikioh@gmail.com> wrote:

> not so rush, of course.
>
>
https://codereview.appspot.**com/8264043/<https://codereview.appspot.com/8264...
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[mikio, 2013-04-04 02:23:29]

> Please file an issue for this, though, so it's not forgotten, and
> cross-link this CL and the issue.

done. issue 5199.

[minux1, 2013-05-20 16:02:22]

https://codereview.appspot.com/8264043/diff/102001/src/pkg/runtime/netpoll_kq...
File src/pkg/runtime/netpoll_kqueue.c (right):

https://codereview.appspot.com/8264043/diff/102001/src/pkg/runtime/netpoll_kq...
src/pkg/runtime/netpoll_kqueue.c:5: // +build darwin freebsd,amd64 freebsd,386,
openbsd
extra comma after "freebsd,386"

the build tags used to exclude freebsd/arm is inconsistent
within this CL.

https://codereview.appspot.com/8264043/diff/102001/src/pkg/runtime/netpoll_kq...
src/pkg/runtime/netpoll_kqueue.c:43: ev[0].udata = (byte*)pd;
on netbsd, kevent.udata is typed uintptr.
perhaps we need to introduce a os-specific typedef
for it.

[mikio, 2013-05-22 13:51:53]

On Tue, May 21, 2013 at 1:02 AM,  <minux.ma@gmail.com> wrote:

> src/pkg/runtime/netpoll_kqueue.c:5: // +build darwin freebsd,amd64
> freebsd,386, openbsd
> extra comma after "freebsd,386"
>
> the build tags used to exclude freebsd/arm is inconsistent
> within this CL.

thx, will update upcoming kqueue cl.

>
https://codereview.appspot.com/8264043/diff/102001/src/pkg/runtime/netpoll_kq...
> src/pkg/runtime/netpoll_kqueue.c:43: ev[0].udata = (byte*)pd;
> on netbsd, kevent.udata is typed uintptr.
> perhaps we need to introduce a os-specific typedef
> for it.

yup, uintptr makes sense.

[minux1, 2013-05-22 19:10:38]

On Wed, May 22, 2013 at 9:51 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:
>
>  >
>
https://codereview.appspot.com/8264043/diff/102001/src/pkg/runtime/netpoll_kq...
> > src/pkg/runtime/netpoll_kqueue.c:43: ev[0].udata = (byte*)pd;
> > on netbsd, kevent.udata is typed uintptr.
> > perhaps we need to introduce a os-specific typedef
> > for it.
>
> yup, uintptr makes sense.
>
but defs_{free,open}bsd_GOARCH.h will use byte* for the type of
kevent.udata,
do you want to manually patch them or introduce a typedef in os_GOOS.h?

[brainman, 2013-08-15 04:33:30]

I take it, these changes overlap with CL 12730043? Lets finish CL 12730043
first. Thank you.

Alex

[mikio, 2013-08-15 05:22:34]

On Thursday, August 15, 2013 1:33:30 PM UTC+9, brainman wrote:

> I take it, these changes overlap with CL 12730043?


Yup, for convenience.
 

> Lets finish CL 12730043 first. Thank you. 
>

Sure.

[mikio, 2013-08-15 07:42:14]

Hello dvyukov@google.com, minux.ma@gmail.com, dave@cheney.net,
bradfitz@golang.org, alex.brainman@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[dvyukov, 2013-08-15 09:37:52]

LGTM

[mikio, 2013-08-15 12:10:13]

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

net: enable runtime-integrated network pollster on freebsd, openbsd

Fixes issue 5199.

Benchmark results on freebsd/amd64 (virtual machine):

benchmark                             old ns/op    new ns/op    delta
BenchmarkTCP4OneShot-2                   184566       187164   +1.41%
BenchmarkTCP4OneShotTimeout-2            215558       187722  -12.91%
BenchmarkTCP4Persistent-2                 59686        41294  -30.81%
BenchmarkTCP4PersistentTimeout-2          60692        39974  -34.14%
BenchmarkTCP6OneShot-2                   226595       223688   -1.28%
BenchmarkTCP6OneShotTimeout-2            253144       225161  -11.05%
BenchmarkTCP6Persistent-2                 69157        55605  -19.60%
BenchmarkTCP6PersistentTimeout-2          70426        53805  -23.60%
BenchmarkTCP4ConcurrentReadWrite-2        53878        56087   +4.10%
BenchmarkTCP6ConcurrentReadWrite-2        66538        68190   +2.48%

benchmark                            old allocs   new allocs    delta
BenchmarkTCP4OneShot-2                       39           36   -7.69%
BenchmarkTCP4OneShotTimeout-2                42           36  -14.29%
BenchmarkTCP4Persistent-2                     1            0  -100.00%
BenchmarkTCP4PersistentTimeout-2              1            0  -100.00%
BenchmarkTCP6OneShot-2                       41           36  -12.20%
BenchmarkTCP6OneShotTimeout-2                43           36  -16.28%
BenchmarkTCP6Persistent-2                     1            0  -100.00%
BenchmarkTCP6PersistentTimeout-2              1            0  -100.00%
BenchmarkTCP4ConcurrentReadWrite-2            0            0     n/a%
BenchmarkTCP6ConcurrentReadWrite-2            0            0     n/a%

benchmark                             old bytes    new bytes    delta
BenchmarkTCP4OneShot-2                     3084         2544  -17.51%
BenchmarkTCP4OneShotTimeout-2              3129         2519  -19.50%
BenchmarkTCP4Persistent-2                    30            0  -100.00%
BenchmarkTCP4PersistentTimeout-2             31            0  -100.00%
BenchmarkTCP6OneShot-2                     3297         2660  -19.32%
BenchmarkTCP6OneShotTimeout-2              3306         2655  -19.69%
BenchmarkTCP6Persistent-2                    31            0  -100.00%
BenchmarkTCP6PersistentTimeout-2             29            0  -100.00%
BenchmarkTCP4ConcurrentReadWrite-2            2            0  -100.00%
BenchmarkTCP6ConcurrentReadWrite-2            7            0  -100.00%

R=dvyukov, minux.ma, dave, bradfitz, alex.brainman
CC=golang-dev
https://codereview.appspot.com/8264043

[rsc, 2013-08-16 01:11:38]

LGTM

Yay!

[rsc, 2013-08-16 01:23:06]

This is the last one, right? I mean, all the operating systems are now
using the new runtime polling, right?

[mikio, 2013-08-16 04:22:04]

On Friday, August 16, 2013 10:23:04 AM UTC+9, rsc wrote:
 

> This is the last one, right? I mean, all the operating systems are now 
> using the new runtime polling, right?
>

nope, freebsd/arm and openbsd/amd64,386,arm remain
but i filed new issue 6146. 

[mikio, 2013-08-16 04:27:46]

s/openbsd/netbsd/

On Friday, August 16, 2013 1:22:03 PM UTC+9, Mikio Hara wrote:
>
> nope, freebsd/arm and openbsd/amd64,386,arm remain
> but i filed new issue 6146. 
>