search

snapview / tokio-tungstenite

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

Implement tungstenite::stream::NoDelay for MaybeTlsStream #263

Closed zazabe closed 1 year ago

zazabe commented 1 year ago

tungstenite::stream::NoDelay is implemented for tungstenite::MaybeTLsStream since v0.15.0 but it's not implemented for the tokio_tungstenite::MaybeTLsStream wrapper, this pull request fix this.

daniel-abramov commented 1 year ago

This is done for a reason. The NoDelay trait is not used inside tokio-tungstenite. There is a possibility to disable the Nagle algorithm though, see: https://github.com/snapview/tokio-tungstenite/pull/252

zazabe commented 1 year ago

Ah ok, but this option to disable Nagle is not released yet, so my workaround currently is to rewrite connect_async_with_config in my crate and call std::net::TcpStream#set_nodelay().

Wondering if it would not be more streamlined with tungstenite API if NoDelay was implemented for tokio_tungstenite::MaybeTLsStream instead of introducing a disable_nagle parameter.

daniel-abramov commented 1 year ago

Yeah, that's right, but we did not do it because according to https://github.com/snapview/tokio-tungstenite/pull/252#issuecomment-1383200591 we figured out that having the Nagle algorithm disabled by default (as we did in tungstenite) is not the best idea as disabling it brings more problems than solves, i.e. tungstenite had it implemented differently, but tungstenite always disabled the Nagle algorithm (which as we found out is not the best idea).