Comment 1:

Sounds good.  Just notes.  SetNonblock calls fcntl, which can block in some cases, so it
would need to call fcntlNB.  And it's hard to convince myself that setsockopt never
blocks, since it has so many different options at different kernel layers, so I think it
would be best to use SetsockoptIntNB as well.

Comment 2:

On linux, setsockopt will trigger a buffer flush using TCP-NODELAY, so I believe it
blocks here.

Comment 3:

A member on the golang-nuts mailing list posted this profile today. I believe
implementing this, even partially, for Write, would improve the reported hello http
benchmark significantly.

Attachments:

1. go_web_server.png (160681 bytes)

Comment 4:

As long as you keep the existing syscall.Write as blocking and only use a new
syscall.WriteNB from code which guarantees the fd is in non-blocking mode (e.g. the net
package).

Comment 5:

Owner changed to @davecheney.

Status changed to Started.

Comment 6:

Patch set http://golang.org/cl/6739043/#ps2001 (not for submission) was
reported to give a 10% improvement by a user on the golang-nuts ML.

Comment 7:

Sendmsg should be handled too. And the Read part too.

Comment 8:

http://golang.org/cl/6813046/

Comment 9:

Labels changed: added size-l.

Comment 10:

Hello,
After investigation making this change from Write to WriteNB does not generate a marked
improvement so I feel that this feature is not currently worth the cost and complexity
of its implementation. 
I have moved the ticket to thinking state and deprioritised the issue to Go1.1 Maybe.
Please correct that status if you feel this is incorrect.
Hopefully, with improvements to the scheduler this feature can be revisited with more
positive results.

Labels changed: added go1.1maybe, removed go1.1.

Owner changed to ---.

Status changed to Thinking.

Comment 11 by songofacandy:

Attached patch makes "Hello World" webapp from 6200req/sec to 7300req/sec on Linux 3.2.0.
(ab -c10 -n20000, go 1.0.3)
Summary of this patch:
* Use accept4 and remove ForkLock
* Use WriteNB and ReadNB
* Currently, only for Linux
Could anyone verify the performance improvement of this patch?

Attachments:

1. accept4.patch (7721 bytes)

Comment 12:

Can you please use a different tool to ab, its use has been discredited. Siege is a good
choice, or httpperf if you can get it to work.

Comment 13 by songofacandy:

command: siege -b -t 10S http://10.10.2.26:8888/
version: SIEGE 2.70
concurrency: 15
w/o GOMAXPROCS
w/o patch 4684.35 trans/sec
with patch 5572.23 trans/sec
with GOMAXPROCS=4
w/o patch 8368.84 trans/sec
with patch 9235.72 trans/sec

Comment 14:

sonofacandy: I think some of the improvements are the accept4 change, which can probably
be submitted independently.
This project uses Rietveld for code review. Can you please spin the accept4 change off
into a separate CL. You will find the contribution guidelines and instructions here,
http://golang.org/doc/contribute.html

Comment 15 by sebastien.paolacci:

I would second Dave's opinion. Accept4() make a lot of sense, and requiring a 2.6.28+
kernel shouldn't be a big issue.
Various tests tend to show that {read,write}NB do not (currently) bring significant
improvements (and possibly slow things down), so I would strip them out of a first patch.
Your current patch doesn't apply cleanly (e.g fd.go doesn't exists anymore): would you
mind just rebasing on tip first (without any refactoring) so we can run further
benchmarks?
Thanks,
Sebastien

Comment 16:

We have either a list of minimum platform specs, or an issue to create such a list for
go 1.1. From memory we require .25 or .27 on intel, but in practice for rhel and Debian
it's actually .32 as that is what ships in their current stable kernels, so I don't
think this is a big issue. 
It will mean Rhel5 moves from unsupported to broken which is a shame, unless we have
some kind of fallback if the syscall fails. I think there is an example of that in the
net package already.

Comment 17:

Re the patch, please move to the Rietveld system.

Comment 18 by songofacandy:

I've created https://golang.org/cl/7188044/

Comment 19:

Done forget to hg mail 7188044

Comment 20 by songofacandy:

I did it. Thank you.

Comment 21:

+1 to not breaking RHEL 5 unless it's absolutely necessary, a fallback if the syscall
fails would be fantastic.

Comment 22:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

Comment 23:

I think it can be closed now. The new scheduler has some basic intelligence to determine
non-blocking syscalls. We can file new issues for specific problems.

Status changed to Fixed.