code review 4272057: net: Don't force epoll/kqueue to wake up in order to ad...

net: Don't force epoll/kqueue to wake up in order to add new events.

In conjunction with the non-blocking system call CL, this
gives about an 8% performance improvement on a client/server
test running on my local machine.

[iant, 2011-03-16 22:55:12]

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

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

[rsc, 2011-03-17 05:19:44]

LGTM

The Linux comments apply to the other systems too.

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd.go
File src/pkg/net/fd.go (right):

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd.go#newcode5
src/pkg/net/fd.go:5: // TODO(rsc): All the prints in this file should go to
standard error.
can delete this

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd.go#newcode92
src/pkg/net/fd.go:92: poll       *pollster // low-level OS hooks
swap poll above Mutex so Mutex controls what's below it

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd.go#newcode129
src/pkg/net/fd.go:129: return
delete

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd_linux.go
File src/pkg/net/fd_linux.go (right):

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd_linux.go#newco...
src/pkg/net/fd_linux.go:44: var ev syscall.EpollEvent
This is a malloc because ev has its address taken below.
Should move into pollster struct and then use

ev := &p.addEv

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd_linux.go#newco...
src/pkg/net/fd_linux.go:71: events, already := p.events[fd]
// pollServer is locked

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd_linux.go#newco...
src/pkg/net/fd_linux.go:88: var ev syscall.EpollEvent
ev := &p.addEv

http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd_linux.go#newco...
src/pkg/net/fd_linux.go:117: var evarray [1]syscall.EpollEvent
Same.  This needs to be a different one because
the lock is not held.

evarray := &p.waitEv

[iant2, 2011-03-17 15:45:11]

rsc@golang.org writes:

>
http://codereview.appspot.com/4272057/diff/3001/src/pkg/net/fd_linux.go#newco...
> src/pkg/net/fd_linux.go:44: var ev syscall.EpollEvent
> This is a malloc because ev has its address taken below.
> Should move into pollster struct and then use
>
> ev := &p.addEv

I was going to do that in a separate CL which makes fd_linux.go like
fd_darwin.go in that it holds an array of events in the pollster struct
rather than calling epoll_wait once per event.  OK to wait for that?

Ian

[rsc, 2011-03-17 15:50:50]

> I was going to do that in a separate CL which makes fd_linux.go like
> fd_darwin.go in that it holds an array of events in the pollster struct
> rather than calling epoll_wait once per event.  OK to wait for that?

Sure.

[iant, 2011-03-17 20:42:43]

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

net: Don't force epoll/kqueue to wake up in order to add new events.

In conjunction with the non-blocking system call CL, this
gives about an 8% performance improvement on a client/server
test running on my local machine.

R=rsc, iant2
CC=golang-dev
http://codereview.appspot.com/4272057