code review 6813046: net: use WriteNB on non-blocking sockets

net: use WriteNB on non-blocking sockets

Update issue 3412.

Requires: https://codereview.appspot.com/7126043/

[dave_cheney.net, 2012-10-28 16:44:53]

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

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

[dave_cheney.net, 2012-10-28 16:46:28]

Cross compilation tests pass on all OSs, but I don't have access to darwin for
*bsd builders at the moment to test this change. 

Assistance from those with darwin or *bsd machines is much appreciated.

[bradfitz, 2012-10-28 16:54:41]

LGTM but you might want to wait for others.

I don't see how this could break darwin.  (which, hah, means it probably
will)

It'd be nice if you could reproduce some benchmark numbers yourself and
include them in the CL description, with repro instructions, or a pointer
to them.  (perhaps in the bug if it's long).  As is it reads like hearsay.


On Sun, Oct 28, 2012 at 5:46 PM, <dave@cheney.net> wrote:

> Cross compilation tests pass on all OSs, but I don't have access to
> darwin for *bsd builders at the moment to test this change.
>
> Assistance from those with darwin or *bsd machines is much appreciated.
>
>
http://codereview.appspot.com/**6813046/<http://codereview.appspot.com/6813046/>
>

[minux1, 2012-10-28 16:56:46]

for such a small change, i think you can just manually generate
zsyscall_GOOS_GOARCH.go files (just pay attention to the relative
order).

https://codereview.appspot.com/6813046/diff/4001/src/pkg/syscall/syscall_darw...
File src/pkg/syscall/syscall_darwin.go (right):

https://codereview.appspot.com/6813046/diff/4001/src/pkg/syscall/syscall_darw...
src/pkg/syscall/syscall_darwin.go:111: func WriteNB(fd int, p []byte) (n int,
err error) { return Write(fd, p) }
please provide docs for WriteNB.
and stress the fact that it can only be used for non-blocking fds.

[dave_cheney.net, 2012-10-28 17:05:28]

> for such a small change, i think you can just manually generate
> zsyscall_GOOS_GOARCH.go files (just pay attention to the relative
> order).

Are you suggesting adding

// sys writeNB(...) = SYS_WRITE

to darwin and *bsd and updating zsyscall*go rather than including these
forwarding methods ?

[r, 2012-10-28 17:14:19]

NOT LGTM

the semantic change is disturbing. networks are used for things other
than http "hello world" benchmarks.

-rob

[minux1, 2012-10-28 17:19:53]

On 2012/10/28 17:05:28, dfc wrote:
> Are you suggesting adding
> 
> // sys writeNB(...) = SYS_WRITE
//sysnb writeNB(...) = SYS_WRITE 
> to darwin and *bsd and updating zsyscall*go rather than including these
> forwarding methods ?
yeah.

[bradfitz, 2012-10-28 17:20:17]

Could you elaborate?

He's not changing the semantics or implementation of syscall.Write.

He's just adding a new syscall function ("WriteNB") that calls the same
system call but tells the go runtime that the file descriptor is already in
non-blocking mode, so scheduling isn't necessary.

This is a performance optimization only (and for all network operations,
not just http!), the same as the non-blocking system calls we already have
SYSNB annotated for.

I don't understand this particular objection.


On Sun, Oct 28, 2012 at 6:14 PM, Rob Pike <r@golang.org> wrote:

> NOT LGTM
>
> the semantic change is disturbing. networks are used for things other
> than http "hello world" benchmarks.
>
> -rob
>

[r, 2012-10-28 17:27:48]

I misunderstood where the magic was happening. I withdraw my objection.

As usual, networking is too complicated but there's not much I can do
about that.

-rob

[dave_cheney.net, 2012-10-28 17:46:37]

Hello mikioh.mikioh@gmail.com, remyoudompheng@gmail.com, bradfitz@golang.org,
minux.ma@gmail.com, r@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[ioe, 2012-10-28 18:11:10]

Just verified it by looking at linux kernel code. Writing to non-blocking 
file descriptors might trigger waiting for things 
like allocating some control structures on system global heavy OOM 
conditions.
The you cannot continue anyway and adding more threads will fail due to 
OOM, too.
But it will not wait on socket memory, but give EAGAIN there, as expected.

On Sunday, October 28, 2012 6:20:19 PM UTC+1, Brad Fitzpatrick wrote:
>
> Could you elaborate?
>
> He's not changing the semantics or implementation of syscall.Write.
>
> He's just adding a new syscall function ("WriteNB") that calls the same 
> system call but tells the go runtime that the file descriptor is already in 
> non-blocking mode, so scheduling isn't necessary.
>
> This is a performance optimization only (and for all network operations, 
> not just http!), the same as the non-blocking system calls we already have 
> SYSNB annotated for.
>
> I don't understand this particular objection.
>
>
> On Sun, Oct 28, 2012 at 6:14 PM, Rob Pike <r...@golang.org
<javascript:>>wrote:
>
>> NOT LGTM
>>
>> the semantic change is disturbing. networks are used for things other
>> than http "hello world" benchmarks.
>>
>> -rob
>>
>
>

[remyoudompheng, 2012-10-28 19:42:05]

I still don't see the corresponding ReadNB

[dave_cheney.net, 2012-10-28 22:09:50]

I would be happy to add this in a follow-up CL.

In the profiles that others have posted Write (and Accept to a smaller
degree) have been the biggest callers of syscall.Syscall, so I'm
tackling those first.

My main concern is having to ask others to perform load tests as I
don't have a large enough test harness to generate a saturating load.

On Mon, Oct 29, 2012 at 6:42 AM,  <remyoudompheng@gmail.com> wrote:
> I still don't see the corresponding ReadNB
>
> http://codereview.appspot.com/6813046/

[bradfitz, 2012-10-29 13:32:31]

I can't find the numbers in that thread.  Or rather, there were a bunch of
numbers, but I can't find which pair was before and after this CL.

You can't just run ab against localhost to measure this? I don't see why you'd
need huge hardware.

[iant, 2012-10-29 16:11:44]

LGTM

https://codereview.appspot.com/6813046/diff/8020/src/pkg/syscall/syscall_unix.go
File src/pkg/syscall/syscall_unix.go (right):

https://codereview.appspot.com/6813046/diff/8020/src/pkg/syscall/syscall_unix...
src/pkg/syscall/syscall_unix.go:146: // WriteNB should only be called when it is
known to not block the
This comment doesn't read quite right to me.  How about something more like:

WriteNB does a write system call to a descriptor that has been set to
non-blocking mode.  Do not call this with a descriptor in blocking mode. 
Blocking mode is the default for all descriptors other than sockets opened by
the net package.

[dave_cheney.net, 2012-10-29 16:13:57]

> WriteNB does a write system call to a descriptor that has been set to
> non-blocking mode.  Do not call this with a descriptor in blocking mode.
>  Blocking mode is the default for all descriptors other than sockets
> opened by the net package.

SGTM, thank you.

@bradfitz. I can do some basic testing with siege but I am concerned
my laptop doesn't have enough horsepower to drive siege and the server
process without interference. Hopefully some of the OPs will be able
to contribute test data from their machines.

[bradfitz, 2012-10-29 16:15:03]

On Mon, Oct 29, 2012 at 5:13 PM, Dave Cheney <dave@cheney.net> wrote:

> > WriteNB does a write system call to a descriptor that has been set to
> > non-blocking mode.  Do not call this with a descriptor in blocking mode.
> >  Blocking mode is the default for all descriptors other than sockets
> > opened by the net package.
>
> SGTM, thank you.
>
> @bradfitz. I can do some basic testing with siege but I am concerned
> my laptop doesn't have enough horsepower to drive siege and the server
> process without interference. Hopefully some of the OPs will be able
> to contribute test data from their machines.
>

Fortunately it's very easy for us to A/B test later, ~1 line change.

[mikio, 2012-10-30 06:36:38]

https://codereview.appspot.com/6813046/diff/11003/src/pkg/syscall/syscall_uni...
File src/pkg/syscall/syscall_unix.go (right):

https://codereview.appspot.com/6813046/diff/11003/src/pkg/syscall/syscall_uni...
src/pkg/syscall/syscall_unix.go:150: func WriteNB(fd int, p []byte) (n int, err
error) {
WriteNB or WriteNonblock; we already have SetNonblock
so maybe the latter might be preferable?

[dave_cheney.net, 2012-10-30 06:50:58]

WriteNB takes its name from the sysnb annotation that
mksyscall.plconsumes. In this case I think it is OK as we aren't
encouraging general
use of this function.
On 30 Oct 2012 07:36, <mikioh.mikioh@gmail.com> wrote:

>
> https://codereview.appspot.**com/6813046/diff/11003/src/**
>
pkg/syscall/syscall_unix.go<https://codereview.appspot.com/6813046/diff/11003/src/pkg/syscall/syscall_unix.go>
> File src/pkg/syscall/syscall_unix.**go (right):
>
> https://codereview.appspot.**com/6813046/diff/11003/src/**
>
pkg/syscall/syscall_unix.go#**newcode150<https://codereview.appspot.com/6813046/diff/11003/src/pkg/syscall/syscall_unix.go#newcode150>
> src/pkg/syscall/syscall_unix.**go:150: func WriteNB(fd int, p []byte) (n
> int, err error) {
> WriteNB or WriteNonblock; we already have SetNonblock
> so maybe the latter might be preferable?
>
>
https://codereview.appspot.**com/6813046/<https://codereview.appspot.com/6813...
>

[mikio, 2012-10-30 06:56:30]

On Tue, Oct 30, 2012 at 3:50 PM, Dave Cheney <dave@cheney.net> wrote:

> WriteNB takes its name from the sysnb annotation that mksyscall.pl consumes.
> In this case I think it is OK as we aren't encouraging general use of this
> function.

makes sense, thank you for explaining it.

[mikio, 2012-10-30 06:58:52]

LGTM

it passes on freebsd/amd64.

[dave_cheney.net, 2012-10-30 07:05:13]

Excellent, thanks for testing.
On 30 Oct 2012 07:58, <mikioh.mikioh@gmail.com> wrote:

> LGTM
>
> it passes on freebsd/amd64.
>
>
https://codereview.appspot.**com/6813046/<https://codereview.appspot.com/6813...
>

[dave_cheney.net, 2012-11-18 04:23:43]

Thank you to those who reviewed this CL. I was traveling at the time so let this
slip, which I apologise for. I will be doing final testing on this CL today and
tomorrow and plan to submit soon after that.

[dave_cheney.net, 2012-11-21 05:31:12]

After (re)reading the siege documentation i've come up with some interesting
results.

Test hardware, Lenovo x220, Core i5, 4Gb ram, Ubuntu 12.10, linux/amd64

Baseline: hg id: 2a55e349097f

Before, GOMAXPROCS unset:
% siege -b -t10s http://127.0.0.1:8080/
** SIEGE 2.70
** Preparing 15 concurrent users for battle.
The server is now under siege...
Lifting the server siege...      done.                                          
                                             Transactions:                 
149320 hits
Availability:                 100.00 %
Elapsed time:                   9.23 secs
Data transferred:               1.85 MB
Response time:                  0.00 secs
Transaction rate:           16177.68 trans/sec
Throughput:                     0.20 MB/sec
Concurrency:                   14.77
Successful transactions:      149320
Failed transactions:               0
Longest transaction:            0.01
Shortest transaction:           0.00

After:
% siege -b -t10s http://127.0.0.1:8080/
** SIEGE 2.70
** Preparing 15 concurrent users for battle.
The server is now under siege...
Lifting the server siege...      done.                                          
                                             Transactions:                  
28436 hits
Availability:                 100.00 %
Elapsed time:                   9.08 secs
Data transferred:               0.35 MB
Response time:                  0.00 secs
Transaction rate:            3131.72 trans/sec
Throughput:                     0.04 MB/sec
Concurrency:                   14.74
Successful transactions:       28437
Failed transactions:               0
Longest transaction:            0.02
Shortest transaction:           0.00

In summary, terrible.

However, 

Before, GOMAXPROCS=4
% siege -b -t10s http://127.0.0.1:8080/
** SIEGE 2.70
** Preparing 15 concurrent users for battle.
The server is now under siege...
Lifting the server siege...      done.                                          
                                             Transactions:                 
154166 hits
Availability:                 100.00 %
Elapsed time:                   9.29 secs
Data transferred:               1.91 MB
Response time:                  0.00 secs
Transaction rate:           16594.83 trans/sec
Throughput:                     0.21 MB/sec
Concurrency:                   14.76
Successful transactions:      154166
Failed transactions:               0
Longest transaction:            0.01
Shortest transaction:           0.00

After:
% siege -b -t10s http://127.0.0.1:8080/
** SIEGE 2.70
** Preparing 15 concurrent users for battle.
The server is now under siege...
Lifting the server siege...      done.                                          
                                             Transactions:                 
149329 hits
Availability:                 100.00 %
Elapsed time:                   9.76 secs
Data transferred:               1.85 MB
Response time:                  0.00 secs
Transaction rate:           15300.10 trans/sec
Throughput:                     0.19 MB/sec
Concurrency:                   14.76
Successful transactions:      149329
Failed transactions:               0
Longest transaction:            0.01
Shortest transaction:           0.00

Now the numbers are comparable, it is possible that with a larger host, say 8,
12 or 16 cores, more throughput could be obtained.

pprof graphs show roughly half of the time spent in syscall.Syscall is now
allocated to syscall.Rawsyscall, as expected.

However, given these results, I am unsure how to proceed as this CL does not
improve the throughput in the basic case, and can easily cause a serious
regression. Granted, this regression may be addressed by GOMAXPROCS tweaking,
but that runs contrary to the ideas of the runtime doing the right thing out of
the box.

[bradfitz, 2012-11-21 06:18:02]

Any change with larger writes?  Those responses look quite small.

Personally I'm not concerned either way if this goes in now vs. later
(after scheduler work).


On Tue, Nov 20, 2012 at 9:31 PM, <dave@cheney.net> wrote:

> After (re)reading the siege documentation i've come up with some
> interesting results.
>
> Test hardware, Lenovo x220, Core i5, 4Gb ram, Ubuntu 12.10, linux/amd64
>
> Baseline: hg id: 2a55e349097f
>
> Before, GOMAXPROCS unset:
> % siege -b -t10s http://127.0.0.1:8080/
> ** SIEGE 2.70
> ** Preparing 15 concurrent users for battle.
> The server is now under siege...
> Lifting the server siege...      done.
>                                                      Transactions:
>            149320 hits
> Availability:                 100.00 %
> Elapsed time:                   9.23 secs
> Data transferred:               1.85 MB
> Response time:                  0.00 secs
> Transaction rate:           16177.68 trans/sec
> Throughput:                     0.20 MB/sec
> Concurrency:                   14.77
> Successful transactions:      149320
> Failed transactions:               0
> Longest transaction:            0.01
> Shortest transaction:           0.00
>
> After:
> % siege -b -t10s http://127.0.0.1:8080/
> ** SIEGE 2.70
> ** Preparing 15 concurrent users for battle.
> The server is now under siege...
> Lifting the server siege...      done.
>                                                      Transactions:
>             28436 hits
> Availability:                 100.00 %
> Elapsed time:                   9.08 secs
> Data transferred:               0.35 MB
> Response time:                  0.00 secs
> Transaction rate:            3131.72 trans/sec
> Throughput:                     0.04 MB/sec
> Concurrency:                   14.74
> Successful transactions:       28437
> Failed transactions:               0
> Longest transaction:            0.02
> Shortest transaction:           0.00
>
> In summary, terrible.
>
> However,
>
> Before, GOMAXPROCS=4
> % siege -b -t10s http://127.0.0.1:8080/
> ** SIEGE 2.70
> ** Preparing 15 concurrent users for battle.
> The server is now under siege...
> Lifting the server siege...      done.
>                                                      Transactions:
>            154166 hits
> Availability:                 100.00 %
> Elapsed time:                   9.29 secs
> Data transferred:               1.91 MB
> Response time:                  0.00 secs
> Transaction rate:           16594.83 trans/sec
> Throughput:                     0.21 MB/sec
> Concurrency:                   14.76
> Successful transactions:      154166
> Failed transactions:               0
> Longest transaction:            0.01
> Shortest transaction:           0.00
>
> After:
> % siege -b -t10s http://127.0.0.1:8080/
> ** SIEGE 2.70
> ** Preparing 15 concurrent users for battle.
> The server is now under siege...
> Lifting the server siege...      done.
>                                                      Transactions:
>            149329 hits
> Availability:                 100.00 %
> Elapsed time:                   9.76 secs
> Data transferred:               1.85 MB
> Response time:                  0.00 secs
> Transaction rate:           15300.10 trans/sec
> Throughput:                     0.19 MB/sec
> Concurrency:                   14.76
> Successful transactions:      149329
> Failed transactions:               0
> Longest transaction:            0.01
> Shortest transaction:           0.00
>
> Now the numbers are comparable, it is possible that with a larger host,
> say 8, 12 or 16 cores, more throughput could be obtained.
>
> pprof graphs show roughly half of the time spent in syscall.Syscall is
> now allocated to syscall.Rawsyscall, as expected.
>
> However, given these results, I am unsure how to proceed as this CL does
> not improve the throughput in the basic case, and can easily cause a
> serious regression. Granted, this regression may be addressed by
> GOMAXPROCS tweaking, but that runs contrary to the ideas of the runtime
> doing the right thing out of the box.
>
>
https://codereview.appspot.**com/6813046/<https://codereview.appspot.com/6813...
>

[remyoudompheng, 2012-11-21 07:43:30]

On 2012/11/21 05:31:12, dfc wrote:
> After (re)reading the siege documentation i've come up with some interesting
> results.
>
> [...]
> 
> pprof graphs show roughly half of the time spent in syscall.Syscall is now
> allocated to syscall.Rawsyscall, as expected.
> 
> However, given these results, I am unsure how to proceed as this CL does not
> improve the throughput in the basic case, and can easily cause a serious
> regression. Granted, this regression may be addressed by GOMAXPROCS tweaking,
> but that runs contrary to the ideas of the runtime doing the right thing out
of
> the box.

How many OS threads do you have before/after the patch ?

[dave_cheney.net, 2012-11-21 07:56:56]

I'm sorry I did not measure that. I was fighting at with running out
of ephemeral ports so only running the test for a short amount of
time.

I will investigate other metrics like CPU/thread usage, and using
larger response bodies tomorrow when I am back on mains power.

On Wed, Nov 21, 2012 at 6:43 PM,  <remyoudompheng@gmail.com> wrote:
> On 2012/11/21 05:31:12, dfc wrote:
>>
>> After (re)reading the siege documentation i've come up with some
>
> interesting
>>
>> results.
>
>
>> [...]
>
>
>> pprof graphs show roughly half of the time spent in syscall.Syscall is
>
> now
>>
>> allocated to syscall.Rawsyscall, as expected.
>
>
>> However, given these results, I am unsure how to proceed as this CL
>
> does not
>>
>> improve the throughput in the basic case, and can easily cause a
>
> serious
>>
>> regression. Granted, this regression may be addressed by GOMAXPROCS
>
> tweaking,
>>
>> but that runs contrary to the ideas of the runtime doing the right
>
> thing out of
>>
>> the box.
>
>
> How many OS threads do you have before/after the patch ?
>
> https://codereview.appspot.com/6813046/

[remyoudompheng, 2012-11-21 21:53:03]

On 2012/11/21 07:56:56, dfc wrote:
> I'm sorry I did not measure that. I was fighting at with running out
> of ephemeral ports so only running the test for a short amount of
> time.

You may find it useful to tweak net.ipv4.tcp_tw_recycle and
net.ipv4.tcp_tw_reuse if needed.

Rémy.

[dave_cheney.net, 2013-01-15 06:12:36]

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.

[bradfitz, 2013-01-15 06:29:45]

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.


On Mon, Jan 14, 2013 at 10:12 PM, <dave@cheney.net> wrote:

> 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/<https://codereview.appspot.com/6813...
>

[dave_cheney.net, 2013-01-15 06:34:27]

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

On Tue, Jan 15, 2013 at 5:29 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> 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.
>
>
> On Mon, Jan 14, 2013 at 10:12 PM, <dave@cheney.net> wrote:
>>
>> 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/
>
>

[remyoudompheng, 2013-01-15 19:30:50]

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

[dvyukov, 2013-01-15 20:15:52]

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...

[dvyukov, 2013-01-15 20:18:16]

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.

[dave_cheney.net, 2013-01-15 20:18:54]

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.

[dvyukov, 2013-01-15 20:19:18]

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

[bradfitz, 2013-01-15 20:43:53]

On Tue, Jan 15, 2013 at 12:15 PM, <dvyukov@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)

[Sebastien Paolacci, 2013-01-15 22:55:50]

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

On Jan 15, 2013, at 9:19 PM, dvyukov@google.com wrote:

> I am surprised that it provides only 10% speedup. Try net benchmarks
> with GOMAXPROCS=1,2,4,8,16...
> 
> https://codereview.appspot.com/6813046/

[dave_cheney.net, 2013-01-16 00:26:08]

Hello mikioh.mikioh@gmail.com, remyoudompheng@gmail.com, bradfitz@golang.org,
minux.ma@gmail.com, r@golang.org, nightlyone@googlemail.com, iant@golang.org,
dvyukov@google.com, sebastien.paolacci@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2013-01-30 16:33:48]

NOT LGTM

I still believe that scheduler changes should make it possible to eliminate this
distinction between blocking and non-blocking system calls. Let's not expose
that wart in any more public API.

[dvyukov, 2013-01-30 16:42:50]

On 2013/01/30 16:33:48, rsc wrote:
> NOT LGTM
> 
> I still believe that scheduler changes should make it possible to eliminate
this
> distinction between blocking and non-blocking system calls. Let's not expose
> that wart in any more public API.

Russ, what do you think about private api for net package?
I need to do more prototyping, but the hypothesis is that it can significantly
simplify initial scheduler implementation. If/when we solve the problem in the
scheduler later, we can remove that apis. So I want to know whether I can count
on non-blocking apis for net, and spent time prototyping scheduler that does not
solve the syscall problem.
Maybe it will not be required, if I find a way to deal with background polling
thread (so that it does not consume CPU all time).

[rsc, 2013-01-30 17:25:58]

Let's wait and see how the scheduler work turns out, please.

Russ

[dave_cheney.net, 2013-01-30 23:48:55]

*** Abandoned ***

[dave_cheney.net, 2013-01-30 23:50:05]

On 2013/01/30 16:33:48, rsc wrote:
> NOT LGTM
> 
> I still believe that scheduler changes should make it possible to eliminate
this
> distinction between blocking and non-blocking system calls. Let's not expose
> that wart in any more public API.

sgtm. I think we've already implemented part of this with the accept4 CL. These
WriteNB changes have not shown to make a constant improvement.