code review 7762044: net: refactoring in preparation for integrated network ...

net: refactoring in preparation for integrated network poller
Introduce pollDesc struct, to split netFD struct into fd-related
and poller-related parts.

[dvyukov, 2013-03-12 18:30:31]

Hello golang-dev@googlegroups.com,

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

[dvyukov, 2013-03-12 18:34:13]

On 2013/03/12 18:30:31, dvyukov wrote:
> Hello mailto:golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://dvyukov%2540google.com%40code.google.com/p/go/

I will appreciate if you look at it now. I want to update and send other
poller-related CLs today.

[bradfitz, 2013-03-12 18:34:24]

https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_poll_unix.go
File src/pkg/net/fd_poll_unix.go (right):

https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_poll_unix.go#...
src/pkg/net/fd_poll_unix.go:37: pollServer *pollServer
Document the mutability of these various fields, and group them.  I think these
first two are immutable.  But then closing and down are mutable.  Well, but
cr/cw are immutable, right?

https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_unix.go
File src/pkg/net/fd_unix.go (right):

https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_unix.go#newco...
src/pkg/net/fd_unix.go:25: closing bool
how's this closing different from pd.closing?

[dvyukov, 2013-03-12 18:38:10]

https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_unix.go
File src/pkg/net/fd_unix.go (right):

https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_unix.go#newco...
src/pkg/net/fd_unix.go:25: closing bool
On 2013/03/12 18:34:25, bradfitz wrote:
> how's this closing different from pd.closing?

They are roughly the same.
The poller now does not use netFD all, all poller-related data is contained
within pollDesc. So it has a copy of closing and sysfd.

The new integrated poller will have just:

type pollDesc struct {
  runtimeCtx uintptr
}

everything else will be inside runtime

[dvyukov, 2013-03-12 18:42:16]

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

Please take another look.

[dvyukov, 2013-03-12 18:43:07]

On 2013/03/12 18:34:24, bradfitz wrote:
> https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_poll_unix.go
> File src/pkg/net/fd_poll_unix.go (right):
> 
>
https://codereview.appspot.com/7762044/diff/7001/src/pkg/net/fd_poll_unix.go#...
> src/pkg/net/fd_poll_unix.go:37: pollServer *pollServer
> Document the mutability of these various fields, and group them.  I think
these
> first two are immutable.  But then closing and down are mutable.  Well, but
> cr/cw are immutable, right?


Done. PTAL

[iant, 2013-03-12 19:49:00]

LGTM

but wait for bradfitz

https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go
File src/pkg/net/fd_poll_unix.go (right):

https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go...
src/pkg/net/fd_poll_unix.go:121: // Evict evicts fd from the pending list,
unblocking
s/fd/pd/ in this paragraph.

[bradfitz, 2013-03-12 19:51:03]

LGTM

https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go
File src/pkg/net/fd_poll_unix.go (right):

https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go...
src/pkg/net/fd_poll_unix.go:36: type pollDesc struct {
Add a little comment on this type before submitting?

[dvyukov, 2013-03-12 20:02:14]

On 2013/03/12 19:51:03, bradfitz wrote:
> LGTM
> 
> https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go
> File src/pkg/net/fd_poll_unix.go (right):
> 
>
https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go...
> src/pkg/net/fd_poll_unix.go:36: type pollDesc struct {
> Add a little comment on this type before submitting?

done

[dvyukov, 2013-03-12 20:02:27]

On 2013/03/12 19:49:00, iant wrote:
> LGTM
> 
> but wait for bradfitz
> 
> https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go
> File src/pkg/net/fd_poll_unix.go (right):
> 
>
https://codereview.appspot.com/7762044/diff/18001/src/pkg/net/fd_poll_unix.go...
> src/pkg/net/fd_poll_unix.go:121: // Evict evicts fd from the pending list,
> unblocking
> s/fd/pd/ in this paragraph.

done

[dvyukov, 2013-03-12 20:03:18]

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

net: refactoring in preparation for integrated network poller
Introduce pollDesc struct, to split netFD struct into fd-related
and poller-related parts.

R=golang-dev, bradfitz, iant
CC=golang-dev
https://codereview.appspot.com/7762044