isobit / ws-tcp-relay

A simple relay between WebSocket clients and TCP servers
MIT License
245 stars 61 forks source link

Use io.CopyBuffer() instead of io.Copy()? #3

Closed echaozh closed 6 years ago

echaozh commented 6 years ago

Thanks for writing a useful tool! I've run into this: https://go-review.googlesource.com/c/go/+/9399 and I think it may be a good idea if you use io.CopyBuffer() to improve the relaying performance.

isobit commented 6 years ago

Interesting, that's not a bad idea. Although I believe the biggest improvement with this would be reducing GC pressure when there are lots of connections opening and closing. Since WebSocket connections are generally longer-lived than say, an HTTP connection, it's not as detrimental to just allocate the copy buffers when each connection is opened (which is what already happens with the io.Copy in copyWorker).

To get any useful performance gain we'll have to implement a proper BufferPool, like in the link you shared - something like this. I'm not entirely sure how to tune the size of the pool and the pre-alloc buf size, though. Those should be tweakable as flags/options but I'd like to pick sane defaults.

isobit commented 6 years ago

Also it's important to note that if the average number of simultaneous connections is greater than the buffer pool size there won't be any benefit in terms of GC, since buffers released by newly closed connections will just be deallocated anyway (unless an unbounded pool is used, which seems like a bad idea).