Re: code review 6813046: net, syscall: use WriteNB on non-blocking sockets (issue 6813046)

[da...@cheney.net]

PTAL. This CL should apply cleanly now. I will try to find some time to
make some new performance measurements. Assistance in doing so is
greatly appreciated.

https://codereview.appspot.com/6813046/

[Brad Fitzpatrick]

If you're nervous about checking this in, I would at least be fine with you checking in all the syscall bits first, so then this CL diff is one line and easier to maintain and cherry-pick into load-testing scenarios later.  (I might be able to find some time to do some, even.)

But I can't imagine this making things worse, if Write in actually non-blocking.  (per your darwin comment)  So I'm actually still fine with you checking this in as-is.

[Dave Cheney]

Thanks Brad. I like the idea of splitting this into two diffs, I'll do that.

[remyoud...@gmail.com]

I'd like we have dvyukov's opinion about the viability of this change.

https://codereview.appspot.com/6813046/

[dvy...@google.com]

Do we actually want to add public functions for that? It's irreversible
change and we will need to explain to users what is NB calls. Eventually
Go can have automatic handling of that...


https://codereview.appspot.com/6813046/

[dvy...@google.com]

On 2013/01/15 20:15:52, dvyukov wrote:
> Do we actually want to add public functions for that? It's
irreversible change
> and we will need to explain to users what is NB calls. Eventually Go
can have
> automatic handling of that...

But I support the idea of using nonblocking syscalls in net package.
I think we also need connect/accept/close and what else is used during
connection establishment, because some servers connect/accept connection
and send only 1 packet over it.



https://codereview.appspot.com/6813046/

[da...@cheney.net]

On 2013/01/15 20:15:52, dvyukov wrote:
> Do we actually want to add public functions for that? It's
irreversible change
> and we will need to explain to users what is NB calls. Eventually Go
can have
> automatic handling of that...

At Brad's suggestion I'm going to break this CL into several parts.

The first will add the WriteNB mechanics to syscall. We can argue about
their usefulness then.

The second will be a small change to net, which switches to WriteNB.
This will be easy to revert if it makes things worse.

Then additional changes to enabled syscall.WriteNB to be non blocking on
all platforms that support it.

https://codereview.appspot.com/6813046/

[dvy...@google.com]

I am surprised that it provides only 10% speedup. Try net benchmarks
with GOMAXPROCS=1,2,4,8,16...

https://codereview.appspot.com/6813046/

[Brad Fitzpatrick]

On Tue, Jan 15, 2013 at 12:15 PM, <dvy...@google.com> wrote:

Do we actually want to add public functions for that? It's irreversible
change and we will need to explain to users what is NB calls. Eventually
Go can have automatic handling of that...

The syscall package is already a gross, scary, inconsistent, largely undocumented place.  Anybody treading in that direction would understand the warning:

// ReadNB is like Read, but does not cause a scheduling event.

// The fd must already be in non-blocking mode or deadlocks may occur.

func ReadNB(fd int, p []byte) (n int, err error)

[Sébastien]

Hello Dmitriy / All,

Few weeks ago I write and play with an edge triggered linux poll server.

For various reasons, having a stripped-down implementation, with a significantly reduced contention surface (and slightly less syscalls), made me hope for some performance improvements.. which happened to be almost just wrong (nothing on 1 core, ~5% on 4-8 cores in the TCPOneShot case and ~20% in the persistent one).

I then blamed the way (supposedly) non-blocking syscalls were not handled, so I flagged accept/connect/write as non-blocking. End result was not better and even slightly worst, either because they did occasionally block (as separately suggested by Iant), or because it happened to adversely disturb the scheduler (by offering less/different resumption points), or because both of them, or because of whatever else.

After few more investigations, what I learn was that even an almost naked poll server was still saturating on the scheduler side, and that the blocking vs non-blocking syscalls point was ultimately not a first order performance consideration.

I incidentally didn't found any pollserver-side way to resorb the abrupt/ crazy slowdowns that still appear beyond 10-12 cores (cf. 6496054), and I still feel tempted by blaming the scheduler for that..

My bottom line would be: if the only remaining issue was to erroneously consider that `connect' (and/or its friends) can block, whilst they don't, then a very significant part of the scheduler would be quite slick... and I would be much more than happy with that situation alone ;).

Best,
Sebastien