From 8838373fadb898a67a164e533eccd3ad9c47adf6 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 10 Feb 2021 18:41:01 +0100 Subject: [PATCH 1/2] src: Limit data frame payload size By limitting the data frame payload size one gains the following beneftis: 1. Reduce delays sending time-sensitive frames, e.g. window updates. 2. Minimize head-of-line blocking across streams. 3. Enable better interleaving of send and receive operations, as each is carried out atomically instead of concurrently with its respective counterpart. Limiting the frame size to 16KiB does not introduce a large overhead. A Yamux frame header is 12 bytes large, thus this change introduces an overhead of ~0.07%. --- src/connection/stream.rs | 1 + src/lib.rs | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/connection/stream.rs b/src/connection/stream.rs index e9290b8b..636f2d5a 100644 --- a/src/connection/stream.rs +++ b/src/connection/stream.rs @@ -330,6 +330,7 @@ impl AsyncWrite for Stream { return Poll::Pending } let k = std::cmp::min(shared.credit as usize, buf.len()); + let k = std::cmp::min(k, crate::MAX_DATA_FRAME_PAYLOAD_SIZE); shared.credit = shared.credit.saturating_sub(k as u32); Vec::from(&buf[.. k]) }; diff --git a/src/lib.rs b/src/lib.rs index 44528100..d67db9dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,6 +40,21 @@ pub use crate::frame::{FrameDecodeError, header::{HeaderDecodeError, StreamId}}; const DEFAULT_CREDIT: u32 = 256 * 1024; // as per yamux specification +/// Maximum number of bytes a Yamux data frame might carry as its payload. +/// +/// The data frame payload size is not restricted by the yamux specification. +/// Still, this implementation restricts the size to: +/// +/// 1. Reduce delays sending time-sensitive frames, e.g. window updates. +/// 2. Minimize head-of-line blocking across streams. +/// 3. Enable better interleaving of send and receive operations, as each is +/// carried out atomically instead of concurrently with its respective +/// counterpart. +/// +/// For details on why this concrete value was chosen, see +/// https://github.com/paritytech/yamux/issues/100. +const MAX_DATA_FRAME_PAYLOAD_SIZE: usize = 16 * 1024; + /// Specifies when window update frames are sent. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum WindowUpdateMode { From 7a77cfbc665d6f88682876bfb9b78f0db063b0c1 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 11 Feb 2021 15:47:57 +0100 Subject: [PATCH 2/2] src/lib: Make split_send_size configurable --- src/connection/stream.rs | 2 +- src/lib.rs | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/connection/stream.rs b/src/connection/stream.rs index 636f2d5a..d35c9ff7 100644 --- a/src/connection/stream.rs +++ b/src/connection/stream.rs @@ -330,7 +330,7 @@ impl AsyncWrite for Stream { return Poll::Pending } let k = std::cmp::min(shared.credit as usize, buf.len()); - let k = std::cmp::min(k, crate::MAX_DATA_FRAME_PAYLOAD_SIZE); + let k = std::cmp::min(k, self.config.split_send_size); shared.credit = shared.credit.saturating_sub(k as u32); Vec::from(&buf[.. k]) }; diff --git a/src/lib.rs b/src/lib.rs index d67db9dc..b2a8740c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,7 +40,8 @@ pub use crate::frame::{FrameDecodeError, header::{HeaderDecodeError, StreamId}}; const DEFAULT_CREDIT: u32 = 256 * 1024; // as per yamux specification -/// Maximum number of bytes a Yamux data frame might carry as its payload. +/// Default maximum number of bytes a Yamux data frame might carry as its +/// payload when being send. Larger Payloads will be split. /// /// The data frame payload size is not restricted by the yamux specification. /// Still, this implementation restricts the size to: @@ -53,7 +54,7 @@ const DEFAULT_CREDIT: u32 = 256 * 1024; // as per yamux specification /// /// For details on why this concrete value was chosen, see /// https://github.com/paritytech/yamux/issues/100. -const MAX_DATA_FRAME_PAYLOAD_SIZE: usize = 16 * 1024; +const DEFAULT_SPLIT_SEND_SIZE: usize = 16 * 1024; /// Specifies when window update frames are sent. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -91,13 +92,15 @@ pub enum WindowUpdateMode { /// - max. number of streams = 8192 /// - window update mode = on receive /// - read after close = true +/// - split send size = 16 KiB #[derive(Debug, Clone)] pub struct Config { receive_window: u32, max_buffer_size: usize, max_num_streams: usize, window_update_mode: WindowUpdateMode, - read_after_close: bool + read_after_close: bool, + split_send_size: usize } impl Default for Config { @@ -107,7 +110,8 @@ impl Default for Config { max_buffer_size: 1024 * 1024, max_num_streams: 8192, window_update_mode: WindowUpdateMode::OnReceive, - read_after_close: true + read_after_close: true, + split_send_size: DEFAULT_SPLIT_SEND_SIZE } } } @@ -148,6 +152,13 @@ impl Config { self.read_after_close = b; self } + + /// Set the max. payload size used when sending data frames. Payloads larger + /// than the configured max. will be split. + pub fn set_split_send_size(&mut self, n: usize) -> &mut Self { + self.split_send_size = n; + self + } } // Check that we can safely cast a `usize` to a `u64`.