search

snapview / tokio-tungstenite

Future-based Tungstenite for Tokio. Lightweight stream-based WebSocket implementation
MIT License
1.88k stars 236 forks source link

connect: disable Nagle algorithm #252

Closed daniel-abramov closed 1 year ago

daniel-abramov commented 1 year ago

Fixes #250.

sdroege commented 1 year ago

That should probably be configurable? I don't think TCP_NODELAY is always the right choice. See e.g. recent https://withinboredom.info/blog/2022/12/29/golang-is-evil-on-shitty-networks/ , or just searching the Internet for it will show lots of people running into problems in various scenarios where that broke things.

daniel-abramov commented 1 year ago

@sdroege, that's a good point (as always)! I've read the article you provided and tried to refresh my knowledge of the Nagle algorithm and indeed it seems like having the TCP_NODELAY off is not that bad. I remember that in tungstenite we had the TCP_NODELAY enabled primarily of the concerns that the smaller packets won't be sent immediately and would cause not only delays, but problems on the receiver side that may not get a message at all if it's very small, but after re-checking the Nagle's algorithm I've figured that it's not the case as it seems like it would buffer the smaller messages only if there are unacknowledged segments while sending them otherwise.

x1957 commented 1 year ago

LGTM

gautamg795 commented 1 year ago

hi, thanks for making this change! would love to get the next release with this change pushed to crates.io so we can start using it in some projects soon :)