-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Raw performance drops by ~25% when using timeouts #626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
What are you using to benchmark this? The issue you linked to was closed because the performance issue was fixed in Go. I just did a benchmark myself and I only get a 2% slowdown with timeouts enabled: https://gist.github.com/erikdubbelboer/0b5468f1a7ba9679b435973b0590d685 |
I've used As said tests were run on openSUSE Tumbleweed, Linux 5.1.7 x64, Go 1.12. The following is the output of
The following snippet was used to build the server:
(I've also tried without The following snippets were used to run and benchmark the server. The complex
The following are the outputs of
|
These are the outputs I'm getting on my Mac book with Without timeouts:
With timeouts:
That's only 2% slower. |
Which code are you using for the server? |
Your exact code. But I have tested it on an Ubuntu machine just now and there I get a 14% slowdown with the timeouts so I guess linux is just much slower with this. Now I'm wondering if we should try to fix this here or open an issue with Go again. |
Unless there is a hidden issue in However, with regard to So, we have the following classes of timeouts:
Thus, as stated in my original comment, we could implement a "background" worker that checks the status of a given connection and if it "took too long" it would just close that connection. Granted this is not very "graceful" to "slam the door" in the client's face, but in the end except a "basic" 400 error code, there isn't anything useful we can provide to the client. Moreover, the source of these timeouts I would say are:
Thus in neither cases, I don't think we should waste CPU, network packets, and especially network buffers to queue an 400 error. Also, one could always implement a "hybrid" timeout approach by enabling either And, speaking about "hybrid" timeouts, another reason we require timeouts is to eliminate dead clients which eat resources under load. Therefore if the server is not loaded, we could "relax" these timeouts. |
I have done another test. I have taken the basic code that fasthttp does to see if I can see a performance difference with very basic code as well: https://gist.github.com/erikdubbelboer/f3d45fe3d04235b9c16fcf3524d54079 On Ubuntu I'm getting:
This is a 9% slowdown which isn't the 14% I'm getting with fasthttp but it does show that Go itself is causing the slowdown. |
I have done some more performance testing and the only difference when I enable I have tried to write my own deadline code and the only fast option I could manage was something like: mu.Lock()
if t, ok := deadlines[conn]; ok {
if !t.Stop() {
<-t.C
}
t.Reset(s.ReadTimeout)
} else {
deadlines[conn] = time.AfterFunc(s.ReadTimeout, func() {
delete(deadlines, conn)
conn.Close()
})
}
mu.Unlock() Having a single goroutine and a list of deadlines is much slower as Go itself is much better at handling the timeouts. |
I've observed that by just enabling
ReadTimeout
,WriteTimeout
andIdleTimeout
the raw performance drops about 25%.I've tested it on a "dummy" server using
fasthttp
:https://github.com/volution/kawipiko/blob/c78613fbf7b81782c0ab9bbb363312d19086d346/sources/cmd/server-dummy.go#L45-L47
By just enabling or disabling those three lines the performance changes.
(I'm using the latest Go 1.12 on Linux 64bit and the latest
fasthttp
code.)I understand that the underlying issue is the
SetDeadline
implementation ofnet.Conn
, and I've even found a Go issue (to which @valyala participated) on this topic:golang/go#25729
However that issue was closed, but the actual problem still persists.
Could we perhaps implement an "alternative" timeout solution, perhaps something on the lines: for each connection we keep an "last touched" timestamp (updated each time we fully read a request, or write a response), and we have a background goroutine that checks these timeouts, and if they have been overrun we just
c.Close()
the connection.It won't be as precise as using
c.SetDeadline
, however it can remove "dead" connections, or "slow-loris" attacks.The text was updated successfully, but these errors were encountered: