From 56a6fd8246443a1838d54a40c5c73f424672be0d Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 20 Mar 2024 14:33:25 +0100 Subject: [PATCH 01/19] perf(bin): add criterion benchmarks Wraps the `neqo-client` and `neqo-server` code, starts the server and runs various benchmarks through the client. Benchmarks: - single-request-1gb - single-request-1mb - requests-per-second - handshakes-per-second --- neqo-bin/Cargo.toml | 16 ++- neqo-bin/benches/main.rs | 103 ++++++++++++++++++ neqo-bin/src/bin/client.rs | 14 +++ neqo-bin/src/bin/server.rs | 14 +++ neqo-bin/src/{bin => }/client/http09.rs | 3 +- neqo-bin/src/{bin => }/client/http3.rs | 2 +- .../src/{bin/client/main.rs => client/mod.rs} | 61 ++++++++--- neqo-bin/src/lib.rs | 40 ++++++- .../src/{bin/server/main.rs => server/mod.rs} | 46 +++++--- neqo-bin/src/{bin => }/server/old_https.rs | 0 neqo-bin/src/udp.rs | 2 + 11 files changed, 262 insertions(+), 39 deletions(-) create mode 100644 neqo-bin/benches/main.rs create mode 100644 neqo-bin/src/bin/client.rs create mode 100644 neqo-bin/src/bin/server.rs rename neqo-bin/src/{bin => }/client/http09.rs (99%) rename neqo-bin/src/{bin => }/client/http3.rs (99%) rename neqo-bin/src/{bin/client/main.rs => client/mod.rs} (91%) rename neqo-bin/src/{bin/server/main.rs => server/mod.rs} (95%) rename neqo-bin/src/{bin => }/server/old_https.rs (100%) diff --git a/neqo-bin/Cargo.toml b/neqo-bin/Cargo.toml index d36d2ecdca..7a5ead47ba 100644 --- a/neqo-bin/Cargo.toml +++ b/neqo-bin/Cargo.toml @@ -11,12 +11,12 @@ license.workspace = true [[bin]] name = "neqo-client" -path = "src/bin/client/main.rs" +path = "src/bin/client.rs" bench = false [[bin]] name = "neqo-server" -path = "src/bin/server/main.rs" +path = "src/bin/server.rs" bench = false [lints] @@ -39,6 +39,18 @@ regex = { version = "1.9", default-features = false, features = ["unicode-perl"] tokio = { version = "1", default-features = false, features = ["net", "time", "macros", "rt", "rt-multi-thread"] } url = { version = "2.5", default-features = false } +[dev-dependencies] +criterion = { version = "0.5", default-features = false, features = ["html_reports", "async_tokio"] } +tokio = { version = "1", default-features = false, features = ["sync"] } + +[features] +bench = [] + [lib] # See https://github.com/bheisler/criterion.rs/blob/master/book/src/faq.md#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options bench = false + +[[bench]] +name = "main" +harness = false +required-features = ["bench"] diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs new file mode 100644 index 0000000000..3053f249dd --- /dev/null +++ b/neqo-bin/benches/main.rs @@ -0,0 +1,103 @@ +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::{path::PathBuf, str::FromStr}; + +use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughput}; +use tokio::runtime::Runtime; + +struct Benchmark { + name: String, + requests: Vec, + /// Download resources in series using separate connections. + download_in_series: bool, + sample_size: Option, +} + +fn transfer(c: &mut Criterion) { + // TODO: Init log level here once https://github.com/mozilla/neqo/pull/1692 is merged. + neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()); + + let done_sender = spawn_server(); + + for Benchmark { + name, + requests, + download_in_series, + sample_size, + } in [ + Benchmark { + name: "single-request-1gb".to_string(), + requests: vec![1024 * 1024 * 1024], + download_in_series: false, + sample_size: Some(10), + }, + Benchmark { + name: "single-request-1mb".to_string(), + requests: vec![1024 * 1024], + download_in_series: false, + sample_size: None, + }, + Benchmark { + name: "requests-per-second(10_000)".to_string(), + // TODO: Reconsider value. + requests: vec![1; 10_000], + download_in_series: false, + sample_size: None, + }, + Benchmark { + name: "handshakes-per-second(100)".to_string(), + // TODO: Reconsider value. + requests: vec![1; 100], + download_in_series: true, + sample_size: None, + }, + ] { + let mut group = c.benchmark_group(name); + group.throughput(if requests[0] > 1 { + Throughput::Bytes(requests[0]) + } else { + Throughput::Elements(requests.len() as u64) + }); + if let Some(size) = sample_size { + group.sample_size(size); + } + group.bench_function("client", |b| { + b.to_async(Runtime::new().unwrap()).iter_batched( + || { + neqo_bin::client::client(neqo_bin::client::Args::new( + &requests, + download_in_series, + )) + }, + |client| async move { + client.await.unwrap(); + }, + BatchSize::PerIteration, + ); + }); + group.finish(); + } + + done_sender.send(()).unwrap(); +} + +fn spawn_server() -> tokio::sync::oneshot::Sender<()> { + let (done_sender, mut done_receiver) = tokio::sync::oneshot::channel(); + std::thread::spawn(move || { + Runtime::new().unwrap().block_on(async { + let mut server = Box::pin(neqo_bin::server::server(neqo_bin::server::Args::default())); + tokio::select! { + _ = &mut done_receiver => {} + _ = &mut server => {} + } + }); + }); + done_sender +} + +criterion_group!(benches, transfer); +criterion_main!(benches); diff --git a/neqo-bin/src/bin/client.rs b/neqo-bin/src/bin/client.rs new file mode 100644 index 0000000000..25c0e8753f --- /dev/null +++ b/neqo-bin/src/bin/client.rs @@ -0,0 +1,14 @@ +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use clap::Parser; + +#[tokio::main] +async fn main() -> Result<(), neqo_bin::client::Error> { + let args = neqo_bin::client::Args::parse(); + + neqo_bin::client::client(args).await +} diff --git a/neqo-bin/src/bin/server.rs b/neqo-bin/src/bin/server.rs new file mode 100644 index 0000000000..8d166c7487 --- /dev/null +++ b/neqo-bin/src/bin/server.rs @@ -0,0 +1,14 @@ +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use clap::Parser; + +#[tokio::main] +async fn main() -> Result<(), std::io::Error> { + let args = neqo_bin::server::Args::parse(); + + neqo_bin::server::server(args).await +} diff --git a/neqo-bin/src/bin/client/http09.rs b/neqo-bin/src/client/http09.rs similarity index 99% rename from neqo-bin/src/bin/client/http09.rs rename to neqo-bin/src/client/http09.rs index 6d9a26fec2..303ac8fc6b 100644 --- a/neqo-bin/src/bin/client/http09.rs +++ b/neqo-bin/src/client/http09.rs @@ -25,8 +25,7 @@ use neqo_transport::{ }; use url::Url; -use super::{get_output_file, Args, KeyUpdateState, Res}; -use crate::qlog_new; +use super::{get_output_file, qlog_new, Args, KeyUpdateState, Res}; pub struct Handler<'a> { streams: HashMap>>, diff --git a/neqo-bin/src/bin/client/http3.rs b/neqo-bin/src/client/http3.rs similarity index 99% rename from neqo-bin/src/bin/client/http3.rs rename to neqo-bin/src/client/http3.rs index 07cc0e4cde..afbcf3b117 100644 --- a/neqo-bin/src/bin/client/http3.rs +++ b/neqo-bin/src/client/http3.rs @@ -26,7 +26,7 @@ use neqo_transport::{ }; use url::Url; -use crate::{get_output_file, qlog_new, Args, KeyUpdateState, Res}; +use super::{get_output_file, qlog_new, Args, KeyUpdateState, Res}; pub(crate) struct Handler<'a> { #[allow( diff --git a/neqo-bin/src/bin/client/main.rs b/neqo-bin/src/client/mod.rs similarity index 91% rename from neqo-bin/src/bin/client/main.rs rename to neqo-bin/src/client/mod.rs index 7b1a5928a6..0055c3e853 100644 --- a/neqo-bin/src/bin/client/main.rs +++ b/neqo-bin/src/client/mod.rs @@ -21,25 +21,26 @@ use futures::{ future::{select, Either}, FutureExt, TryFutureExt, }; -use neqo_bin::udp; use neqo_common::{self as common, qdebug, qinfo, qlog::NeqoQlog, Datagram, Role}; use neqo_crypto::{ constants::{TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256}, init, Cipher, ResumptionToken, }; -use neqo_http3::{Error, Output}; +use neqo_http3::Output; use neqo_transport::{AppError, ConnectionId, Error as TransportError, Version}; use qlog::{events::EventImportance, streamer::QlogStreamer}; use tokio::time::Sleep; use url::{Origin, Url}; +use crate::{udp, SharedArgs}; + mod http09; mod http3; const BUFWRITER_BUFFER_SIZE: usize = 64 * 1024; #[derive(Debug)] -pub enum ClientError { +pub enum Error { ArgumentError(&'static str), Http3Error(neqo_http3::Error), IoError(io::Error), @@ -47,40 +48,40 @@ pub enum ClientError { TransportError(neqo_transport::Error), } -impl From for ClientError { +impl From for Error { fn from(err: io::Error) -> Self { Self::IoError(err) } } -impl From for ClientError { +impl From for Error { fn from(err: neqo_http3::Error) -> Self { Self::Http3Error(err) } } -impl From for ClientError { +impl From for Error { fn from(_err: qlog::Error) -> Self { Self::QlogError } } -impl From for ClientError { +impl From for Error { fn from(err: neqo_transport::Error) -> Self { Self::TransportError(err) } } -impl Display for ClientError { +impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "Error: {self:?}")?; Ok(()) } } -impl std::error::Error for ClientError {} +impl std::error::Error for Error {} -type Res = Result; +type Res = Result; /// Track whether a key update is needed. #[derive(Debug, PartialEq, Eq)] @@ -90,14 +91,14 @@ impl KeyUpdateState { pub fn maybe_update(&mut self, update_fn: F) -> Res<()> where F: FnOnce() -> Result<(), E>, - E: Into, + E: Into, { if self.0 { if let Err(e) = update_fn() { let e = e.into(); match e { - ClientError::TransportError(TransportError::KeyUpdateBlocked) - | ClientError::Http3Error(Error::TransportError( + Error::TransportError(TransportError::KeyUpdateBlocked) + | Error::Http3Error(neqo_http3::Error::TransportError( TransportError::KeyUpdateBlocked, )) => (), _ => return Err(e), @@ -120,7 +121,7 @@ impl KeyUpdateState { #[allow(clippy::struct_excessive_bools)] // Not a good use of that lint. pub struct Args { #[command(flatten)] - shared: neqo_bin::SharedArgs, + shared: SharedArgs, urls: Vec, @@ -182,6 +183,34 @@ pub struct Args { } impl Args { + #[must_use] + #[cfg(feature = "bench")] + #[allow(clippy::missing_panics_doc)] + pub fn new(requests: &[u64], download_in_series: bool) -> Self { + use std::str::FromStr; + Self { + shared: crate::SharedArgs::default(), + urls: requests + .iter() + .map(|r| Url::from_str(&format!("http://127.0.0.1:12345/{r}")).unwrap()) + .collect(), + method: "GET".into(), + header: vec![], + max_concurrent_push_streams: 10, + download_in_series, + concurrency: 100, + output_read_data: false, + output_dir: Some("/tmp/out".into()), + resume: false, + key_update: false, + ech: None, + ipv4_only: false, + ipv6_only: false, + test: None, + upload_size: 100, + } + } + fn get_ciphers(&self) -> Vec { self.shared .ciphers @@ -434,11 +463,9 @@ fn qlog_new(args: &Args, hostname: &str, cid: &ConnectionId) -> Res { } } -#[tokio::main] -async fn main() -> Res<()> { +pub async fn client(mut args: Args) -> Res<()> { init(); - let mut args = Args::parse(); args.update_for_tests(); let urls_by_origin = args diff --git a/neqo-bin/src/lib.rs b/neqo-bin/src/lib.rs index b7bc158245..380c56ddce 100644 --- a/neqo-bin/src/lib.rs +++ b/neqo-bin/src/lib.rs @@ -4,6 +4,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![allow(clippy::missing_panics_doc)] +#![allow(clippy::missing_errors_doc)] + use std::{ fmt::{self, Display}, net::{SocketAddr, ToSocketAddrs}, @@ -17,7 +20,9 @@ use neqo_transport::{ Version, }; -pub mod udp; +pub mod client; +pub mod server; +mod udp; #[derive(Debug, Parser)] pub struct SharedArgs { @@ -57,6 +62,23 @@ pub struct SharedArgs { pub quic_parameters: QuicParameters, } +#[cfg(feature = "bench")] +impl Default for SharedArgs { + fn default() -> Self { + Self { + alpn: "h3".into(), + qlog_dir: None, + max_table_size_encoder: 16384, + max_table_size_decoder: 16384, + max_blocked_streams: 10, + ciphers: vec![], + qns_test: None, + use_old_http: false, + quic_parameters: QuicParameters::default(), + } + } +} + #[derive(Debug, Parser)] pub struct QuicParameters { #[arg( @@ -102,6 +124,22 @@ pub struct QuicParameters { pub preferred_address_v6: Option, } +#[cfg(feature = "bench")] +impl Default for QuicParameters { + fn default() -> Self { + Self { + quic_version: vec![], + max_streams_bidi: 16, + max_streams_uni: 16, + idle_timeout: 30, + congestion_control: CongestionControlAlgorithm::NewReno, + no_pacing: false, + preferred_address_v4: None, + preferred_address_v6: None, + } + } +} + impl QuicParameters { fn get_sock_addr(opt: &Option, v: &str, f: F) -> Option where diff --git a/neqo-bin/src/bin/server/main.rs b/neqo-bin/src/server/mod.rs similarity index 95% rename from neqo-bin/src/bin/server/main.rs rename to neqo-bin/src/server/mod.rs index f694cf98c1..eacf32603f 100644 --- a/neqo-bin/src/bin/server/main.rs +++ b/neqo-bin/src/server/mod.rs @@ -24,28 +24,28 @@ use futures::{ future::{select, select_all, Either}, FutureExt, }; -use neqo_bin::udp; use neqo_common::{hex, qinfo, qwarn, Datagram, Header}; use neqo_crypto::{ constants::{TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256}, generate_ech_keys, init_db, random, AntiReplay, Cipher, }; use neqo_http3::{ - Error, Http3OrWebTransportStream, Http3Parameters, Http3Server, Http3ServerEvent, StreamId, + Http3OrWebTransportStream, Http3Parameters, Http3Server, Http3ServerEvent, StreamId, }; use neqo_transport::{ server::ValidateAddress, ConnectionIdGenerator, Output, RandomConnectionIdGenerator, Version, }; +use old_https::Http09Server; use tokio::time::Sleep; -use crate::old_https::Http09Server; +use crate::{udp, SharedArgs}; const ANTI_REPLAY_WINDOW: Duration = Duration::from_secs(10); mod old_https; #[derive(Debug)] -pub enum ServerError { +pub enum Error { ArgumentError(&'static str), Http3Error(neqo_http3::Error), IoError(io::Error), @@ -53,44 +53,44 @@ pub enum ServerError { TransportError(neqo_transport::Error), } -impl From for ServerError { +impl From for Error { fn from(err: io::Error) -> Self { Self::IoError(err) } } -impl From for ServerError { +impl From for Error { fn from(err: neqo_http3::Error) -> Self { Self::Http3Error(err) } } -impl From for ServerError { +impl From for Error { fn from(_err: qlog::Error) -> Self { Self::QlogError } } -impl From for ServerError { +impl From for Error { fn from(err: neqo_transport::Error) -> Self { Self::TransportError(err) } } -impl Display for ServerError { +impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "Error: {self:?}")?; Ok(()) } } -impl std::error::Error for ServerError {} +impl std::error::Error for Error {} #[derive(Debug, Parser)] #[command(author, version, about, long_about = None)] -struct Args { +pub struct Args { #[command(flatten)] - shared: neqo_bin::SharedArgs, + shared: SharedArgs, /// List of IP:port to listen on #[arg(default_value = "[::]:4433")] @@ -115,6 +115,22 @@ struct Args { ech: bool, } +#[cfg(feature = "bench")] +impl Default for Args { + fn default() -> Self { + use std::str::FromStr; + + Self { + shared: crate::SharedArgs::default(), + hosts: vec!["[::]:12345".to_string()], + db: PathBuf::from_str("../test-fixture/db").unwrap(), + key: "key".to_string(), + retry: false, + ech: false, + } + } +} + impl Args { fn get_ciphers(&self) -> Vec { self.shared @@ -342,7 +358,7 @@ impl HttpServer for SimpleServer { } } else { stream - .cancel_fetch(Error::HttpRequestIncomplete.code()) + .cancel_fetch(neqo_http3::Error::HttpRequestIncomplete.code()) .unwrap(); continue; }; @@ -568,11 +584,9 @@ enum Ready { Timeout, } -#[tokio::main] -async fn main() -> Result<(), io::Error> { +pub async fn server(mut args: Args) -> Result<(), io::Error> { const HQ_INTEROP: &str = "hq-interop"; - let mut args = Args::parse(); assert!(!args.key.is_empty(), "Need at least one key"); init_db(args.db.clone()); diff --git a/neqo-bin/src/bin/server/old_https.rs b/neqo-bin/src/server/old_https.rs similarity index 100% rename from neqo-bin/src/bin/server/old_https.rs rename to neqo-bin/src/server/old_https.rs diff --git a/neqo-bin/src/udp.rs b/neqo-bin/src/udp.rs index f4ede0b5c2..7ccfa1f36f 100644 --- a/neqo-bin/src/udp.rs +++ b/neqo-bin/src/udp.rs @@ -23,6 +23,8 @@ use tokio::io::Interest; const RECV_BUF_SIZE: usize = u16::MAX as usize; pub struct Socket { + #[allow(unknown_lints)] // available with Rust v1.75 + #[allow(clippy::struct_field_names)] socket: tokio::net::UdpSocket, state: UdpSocketState, recv_buf: Vec, From 2be3b1d58d971455683f9a209026e1c09ff5d15d Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 23 Mar 2024 16:10:54 +0100 Subject: [PATCH 02/19] Rename benchmark instances --- neqo-bin/benches/main.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index 3053f249dd..3033c83097 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -30,27 +30,25 @@ fn transfer(c: &mut Criterion) { sample_size, } in [ Benchmark { - name: "single-request-1gb".to_string(), + name: "1-conn/1-1gb-resp".to_string(), requests: vec![1024 * 1024 * 1024], download_in_series: false, sample_size: Some(10), }, Benchmark { - name: "single-request-1mb".to_string(), - requests: vec![1024 * 1024], + name: "1-conn/1-100mb-resp".to_string(), + requests: vec![100 * 1024 * 1024], download_in_series: false, - sample_size: None, + sample_size: Some(10), }, Benchmark { - name: "requests-per-second(10_000)".to_string(), - // TODO: Reconsider value. + name: "1-conn/10_000-1b-seq-resp (aka. RPS)".to_string(), requests: vec![1; 10_000], download_in_series: false, sample_size: None, }, Benchmark { - name: "handshakes-per-second(100)".to_string(), - // TODO: Reconsider value. + name: "100-seq-conn/1-1b-resp (aka. HPS)".to_string(), requests: vec![1; 100], download_in_series: true, sample_size: None, @@ -58,6 +56,7 @@ fn transfer(c: &mut Criterion) { ] { let mut group = c.benchmark_group(name); group.throughput(if requests[0] > 1 { + assert_eq!(requests.len(), 1); Throughput::Bytes(requests[0]) } else { Throughput::Elements(requests.len() as u64) From 1d59921675de6d3cdc8c6e65bacc4e301b7c3c11 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 23 Mar 2024 16:11:10 +0100 Subject: [PATCH 03/19] Turn off logging --- neqo-bin/benches/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index 3033c83097..3d0d7d3594 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -18,6 +18,7 @@ struct Benchmark { } fn transfer(c: &mut Criterion) { + neqo_common::log::init(Some(log::LevelFilter::Off)); // TODO: Init log level here once https://github.com/mozilla/neqo/pull/1692 is merged. neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()); From af0637533cfc5e575ca5d1dda2fba71672dfdcfa Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 23 Mar 2024 16:18:43 +0100 Subject: [PATCH 04/19] Remove 100mb sample size restriction --- neqo-bin/benches/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index 3d0d7d3594..e7a7bd8c99 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -40,7 +40,7 @@ fn transfer(c: &mut Criterion) { name: "1-conn/1-100mb-resp".to_string(), requests: vec![100 * 1024 * 1024], download_in_series: false, - sample_size: Some(10), + sample_size: None, }, Benchmark { name: "1-conn/10_000-1b-seq-resp (aka. RPS)".to_string(), From 0fe6078758fac21ae8834ed894d963467a5c7dbd Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 23 Mar 2024 16:50:42 +0100 Subject: [PATCH 05/19] Use v6 --- neqo-bin/src/client/mod.rs | 2 +- neqo-bin/src/server/mod.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs index 69dee11c48..ff5377efaa 100644 --- a/neqo-bin/src/client/mod.rs +++ b/neqo-bin/src/client/mod.rs @@ -200,7 +200,7 @@ impl Args { shared: crate::SharedArgs::default(), urls: requests .iter() - .map(|r| Url::from_str(&format!("http://127.0.0.1:12345/{r}")).unwrap()) + .map(|r| Url::from_str(&format!("http://[::1]:12345/{r}")).unwrap()) .collect(), method: "GET".into(), header: vec![], diff --git a/neqo-bin/src/server/mod.rs b/neqo-bin/src/server/mod.rs index 0ba595091e..a61fd94c53 100644 --- a/neqo-bin/src/server/mod.rs +++ b/neqo-bin/src/server/mod.rs @@ -122,7 +122,6 @@ pub struct Args { impl Default for Args { fn default() -> Self { use std::str::FromStr; - Self { verbose: clap_verbosity_flag::Verbosity::::default(), shared: crate::SharedArgs::default(), From d450df0cc69ae5c66bca5ee812a6baf55c7d9ff2 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sun, 24 Mar 2024 09:32:29 +0100 Subject: [PATCH 06/19] Remove 1gb It just takes too long on the bench machine --- neqo-bin/benches/main.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index e7a7bd8c99..a0a1fe9c46 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -30,17 +30,11 @@ fn transfer(c: &mut Criterion) { download_in_series, sample_size, } in [ - Benchmark { - name: "1-conn/1-1gb-resp".to_string(), - requests: vec![1024 * 1024 * 1024], - download_in_series: false, - sample_size: Some(10), - }, Benchmark { name: "1-conn/1-100mb-resp".to_string(), requests: vec![100 * 1024 * 1024], download_in_series: false, - sample_size: None, + sample_size: Some(10), }, Benchmark { name: "1-conn/10_000-1b-seq-resp (aka. RPS)".to_string(), From d4fcce1ffa8c006e460f78d48daf7587cff80a95 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sun, 24 Mar 2024 09:36:29 +0100 Subject: [PATCH 07/19] Use /dev/null --- neqo-bin/benches/main.rs | 3 +-- neqo-bin/src/client/mod.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index a0a1fe9c46..3d38abef0c 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -19,7 +19,6 @@ struct Benchmark { fn transfer(c: &mut Criterion) { neqo_common::log::init(Some(log::LevelFilter::Off)); - // TODO: Init log level here once https://github.com/mozilla/neqo/pull/1692 is merged. neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()); let done_sender = spawn_server(); @@ -31,7 +30,7 @@ fn transfer(c: &mut Criterion) { sample_size, } in [ Benchmark { - name: "1-conn/1-100mb-resp".to_string(), + name: "1-conn/1-100mb-resp (aka. Download)".to_string(), requests: vec![100 * 1024 * 1024], download_in_series: false, sample_size: Some(10), diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs index ff5377efaa..e0169e3f24 100644 --- a/neqo-bin/src/client/mod.rs +++ b/neqo-bin/src/client/mod.rs @@ -208,7 +208,7 @@ impl Args { download_in_series, concurrency: 100, output_read_data: false, - output_dir: Some("/tmp/out".into()), + output_dir: Some("/dev/null".into()), resume: false, key_update: false, ech: None, From 0c359aa8de7e2795375566ab2e56f1db474b3014 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sun, 24 Mar 2024 09:46:49 +0100 Subject: [PATCH 08/19] rework imports --- neqo-bin/benches/main.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index 3d38abef0c..1fc3c87a05 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -7,6 +7,8 @@ use std::{path::PathBuf, str::FromStr}; use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughput}; +use neqo_bin::{client, server}; +use neqo_common::log; use tokio::runtime::Runtime; struct Benchmark { @@ -18,7 +20,7 @@ struct Benchmark { } fn transfer(c: &mut Criterion) { - neqo_common::log::init(Some(log::LevelFilter::Off)); + log::init(Some(log::LevelFilter::Off)); neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()); let done_sender = spawn_server(); @@ -60,12 +62,7 @@ fn transfer(c: &mut Criterion) { } group.bench_function("client", |b| { b.to_async(Runtime::new().unwrap()).iter_batched( - || { - neqo_bin::client::client(neqo_bin::client::Args::new( - &requests, - download_in_series, - )) - }, + || client::client(client::Args::new(&requests, download_in_series)), |client| async move { client.await.unwrap(); }, @@ -82,7 +79,7 @@ fn spawn_server() -> tokio::sync::oneshot::Sender<()> { let (done_sender, mut done_receiver) = tokio::sync::oneshot::channel(); std::thread::spawn(move || { Runtime::new().unwrap().block_on(async { - let mut server = Box::pin(neqo_bin::server::server(neqo_bin::server::Args::default())); + let mut server = Box::pin(server::server(server::Args::default())); tokio::select! { _ = &mut done_receiver => {} _ = &mut server => {} From 44cb987ab579695d25bee0cc5abaa86ce91444f2 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sun, 24 Mar 2024 09:56:14 +0100 Subject: [PATCH 09/19] Fix import --- neqo-bin/benches/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs index 1fc3c87a05..fe3aba2714 100644 --- a/neqo-bin/benches/main.rs +++ b/neqo-bin/benches/main.rs @@ -8,7 +8,6 @@ use std::{path::PathBuf, str::FromStr}; use criterion::{criterion_group, criterion_main, BatchSize, Criterion, Throughput}; use neqo_bin::{client, server}; -use neqo_common::log; use tokio::runtime::Runtime; struct Benchmark { @@ -20,7 +19,7 @@ struct Benchmark { } fn transfer(c: &mut Criterion) { - log::init(Some(log::LevelFilter::Off)); + neqo_common::log::init(Some(log::LevelFilter::Off)); neqo_crypto::init_db(PathBuf::from_str("../test-fixture/db").unwrap()); let done_sender = spawn_server(); From a0ef46a3d321aa00aae0d3835ef44533dded46bf Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 26 Mar 2024 14:27:25 +0100 Subject: [PATCH 10/19] Test without CPU pinning --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 134f78f559..6fff63e1ad 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -55,7 +55,7 @@ jobs: # Pin the benchmark run to core 0 and run all benchmarks at elevated priority. - name: Run cargo bench run: | - taskset -c 0 nice -n -20 \ + nice -n -20 \ cargo "+$TOOLCHAIN" bench --features bench -- --noplot | tee results.txt # Pin the transfer benchmark to core 0 and run it at elevated priority inside perf. From e9882872684242dea81d18a0db2f045bb9da1ae2 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 26 Mar 2024 15:00:41 +0100 Subject: [PATCH 11/19] Revert "Test without CPU pinning" This reverts commit a0ef46a3d321aa00aae0d3835ef44533dded46bf. --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 7e10a05174..80c51c236d 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -73,7 +73,7 @@ jobs: # Pin the benchmark run to core 0 and run all benchmarks at elevated priority. - name: Run cargo bench run: | - nice -n -20 \ + taskset -c 0 nice -n -20 \ cargo "+$TOOLCHAIN" bench --features bench -- --noplot | tee results.txt # Compare various configurations of neqo against msquic, and gather perf data From 6648d1531d4305ef3f316d64136a1271b4b21499 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 26 Mar 2024 15:52:49 +0100 Subject: [PATCH 12/19] Pin all but neqo-bin to CPU 0 --- .github/workflows/bench.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 80c51c236d..cc7ee828f4 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -70,11 +70,19 @@ jobs: - name: Prepare machine run: sudo /root/bin/prep.sh - # Pin the benchmark run to core 0 and run all benchmarks at elevated priority. - name: Run cargo bench run: | - taskset -c 0 nice -n -20 \ - cargo "+$TOOLCHAIN" bench --features bench -- --noplot | tee results.txt + # Pin all but neqo-bin benchmarks to CPU 0. + benchmarks=( + ("taskset -c 0" "--exclude neqo-bin") + ("" "--package neqo-bin") + ) + + for config in "${benchmarks[@]}"; do + read -r cpu_pinning packages <<< "${config}" + # run all benchmarks at elevated priority. + $cpu_pinning nice -n -20 cargo "+$TOOLCHAIN" bench $packages --features bench -- --noplot | tee -a results.txt + done # Compare various configurations of neqo against msquic, and gather perf data # during the hyperfine runs. From 4981f0351998c56f8a2b0078742b7f90849df706 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 26 Mar 2024 15:59:14 +0100 Subject: [PATCH 13/19] Quote package --- .github/workflows/bench.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index cc7ee828f4..bc5a272760 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -80,8 +80,8 @@ jobs: for config in "${benchmarks[@]}"; do read -r cpu_pinning packages <<< "${config}" - # run all benchmarks at elevated priority. - $cpu_pinning nice -n -20 cargo "+$TOOLCHAIN" bench $packages --features bench -- --noplot | tee -a results.txt + # Run all benchmarks at elevated priority. + $cpu_pinning nice -n -20 cargo "+$TOOLCHAIN" bench "$packages" --features bench -- --noplot | tee -a results.txt done # Compare various configurations of neqo against msquic, and gather perf data From cbf3a5e767d28628b63de4ca4427043019d2be14 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 26 Mar 2024 16:20:54 +0100 Subject: [PATCH 14/19] Add rational for neqo-bin handling --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index bc5a272760..1124d22b0f 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -72,7 +72,7 @@ jobs: - name: Run cargo bench run: | - # Pin all but neqo-bin benchmarks to CPU 0. + # Pin all but neqo-bin benchmarks to CPU 0. neqo-bin benchmarks run both a client and a server, thus benefiting from multiple CPU core. benchmarks=( ("taskset -c 0" "--exclude neqo-bin") ("" "--package neqo-bin") From 3541d179c1778ee27e89b806cf15d829e306dd60 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 27 Mar 2024 09:30:52 +0100 Subject: [PATCH 15/19] Rework tuples --- .github/workflows/bench.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 1124d22b0f..6750a73985 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -74,14 +74,13 @@ jobs: run: | # Pin all but neqo-bin benchmarks to CPU 0. neqo-bin benchmarks run both a client and a server, thus benefiting from multiple CPU core. benchmarks=( - ("taskset -c 0" "--exclude neqo-bin") - ("" "--package neqo-bin") + "taskset -c 0,--exclude neqo-bin" + ",--package neqo-bin" ) - for config in "${benchmarks[@]}"; do - read -r cpu_pinning packages <<< "${config}" + IFS=',' read -r cpu_pinning packages <<< "$config" # Run all benchmarks at elevated priority. - $cpu_pinning nice -n -20 cargo "+$TOOLCHAIN" bench "$packages" --features bench -- --noplot | tee -a results.txt + nice -n -20 "$cpu_pinning" cargo "+$TOOLCHAIN" bench "$packages" --features bench -- --noplot | tee -a results.txt done # Compare various configurations of neqo against msquic, and gather perf data From 86781882a9c51b64e6b2c03b9f3738371df6a54a Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 27 Mar 2024 13:02:30 +0100 Subject: [PATCH 16/19] Pin first --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 6750a73985..471a8dd85e 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -80,7 +80,7 @@ jobs: for config in "${benchmarks[@]}"; do IFS=',' read -r cpu_pinning packages <<< "$config" # Run all benchmarks at elevated priority. - nice -n -20 "$cpu_pinning" cargo "+$TOOLCHAIN" bench "$packages" --features bench -- --noplot | tee -a results.txt + $cpu_pinning nice -n -20 cargo "+$TOOLCHAIN" bench "$packages" --features bench -- --noplot | tee -a results.txt done # Compare various configurations of neqo against msquic, and gather perf data From 6eaea5c1da935a06fdbce36bb4396dd50b34cf20 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 27 Mar 2024 13:06:15 +0100 Subject: [PATCH 17/19] Just duplicate the two calls --- .github/workflows/bench.yml | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 471a8dd85e..bf0cdcb4af 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -72,16 +72,11 @@ jobs: - name: Run cargo bench run: | + # Run all benchmarks at elevated priority. # Pin all but neqo-bin benchmarks to CPU 0. neqo-bin benchmarks run both a client and a server, thus benefiting from multiple CPU core. - benchmarks=( - "taskset -c 0,--exclude neqo-bin" - ",--package neqo-bin" - ) - for config in "${benchmarks[@]}"; do - IFS=',' read -r cpu_pinning packages <<< "$config" - # Run all benchmarks at elevated priority. - $cpu_pinning nice -n -20 cargo "+$TOOLCHAIN" bench "$packages" --features bench -- --noplot | tee -a results.txt - done + taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --exclude neqo-bin --features bench -- --noplot | tee results.txt + taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt + # Compare various configurations of neqo against msquic, and gather perf data # during the hyperfine runs. From c637266e98ce5986541f3fc08ff404a77c2beba5 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 27 Mar 2024 13:52:49 +0100 Subject: [PATCH 18/19] Add --workspace flag --- .github/workflows/bench.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index bf0cdcb4af..962d50f3b7 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -72,9 +72,11 @@ jobs: - name: Run cargo bench run: | + # Pin all but neqo-bin benchmarks to CPU 0. neqo-bin benchmarks run + # both a client and a server, thus benefiting from multiple CPU cores. + # # Run all benchmarks at elevated priority. - # Pin all but neqo-bin benchmarks to CPU 0. neqo-bin benchmarks run both a client and a server, thus benefiting from multiple CPU core. - taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --exclude neqo-bin --features bench -- --noplot | tee results.txt + taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --workspace --exclude neqo-bin --features bench -- --noplot | tee results.txt taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt From 27ae7cbeb21d1858d3e371fa3a3ec01d029a93cd Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 27 Mar 2024 15:30:41 +0100 Subject: [PATCH 19/19] Remove taskset from neqo-bin --- .github/workflows/bench.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 962d50f3b7..5df8bcfd91 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -77,7 +77,7 @@ jobs: # # Run all benchmarks at elevated priority. taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --workspace --exclude neqo-bin --features bench -- --noplot | tee results.txt - taskset -c 0 nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt + nice -n -20 cargo "+$TOOLCHAIN" bench --package neqo-bin --features bench -- --noplot | tee -a results.txt # Compare various configurations of neqo against msquic, and gather perf data