Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor benching harness to separate out client and server connections #4113

Merged
merged 9 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions bindings/rust/bench/.cargo/config.toml

This file was deleted.

5 changes: 3 additions & 2 deletions bindings/rust/bench/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ historical-perf = []
s2n-tls = { path = "../s2n-tls" }
rustls = "0.21"
rustls-pemfile = "1.0"
openssl = "0.10"
openssl = { version = "0.10", features = ["vendored"] }
errno = "0.3"
libc = "0.2"
crabgrind = "0.1"
Expand All @@ -19,9 +19,10 @@ rand_distr = "0.4"
plotters = "0.3"
serde_json = "1.0"
semver = "1.0"
strum = { version = "0.25", features = ["derive"] }

[dev-dependencies]
criterion = "0.3"
criterion = "0.5"

[[bench]]
name = "handshake"
Expand Down
51 changes: 22 additions & 29 deletions bindings/rust/bench/benches/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,77 +2,70 @@
// SPDX-License-Identifier: Apache-2.0

use bench::{
CipherSuite, CryptoConfig,
ECGroup::{self, *},
HandshakeType::{self, *},
OpenSslHarness, RustlsHarness, S2NHarness,
SigType::{self, *},
TlsBenchHarness,
CipherSuite, CryptoConfig, HandshakeType, KXGroup, OpenSslConnection, RustlsConnection,
S2NConnection, SigType, TlsConnPair, TlsConnection,
};
use criterion::{
criterion_group, criterion_main, measurement::WallTime, BatchSize, BenchmarkGroup, Criterion,
};
use strum::IntoEnumIterator;

pub fn bench_handshake_params(c: &mut Criterion) {
fn bench_handshake_for_library<T: TlsBenchHarness>(
fn bench_handshake_for_library<T: TlsConnection>(
tinzh marked this conversation as resolved.
Show resolved Hide resolved
bench_group: &mut BenchmarkGroup<WallTime>,
name: &str,
handshake_type: HandshakeType,
ec_group: ECGroup,
kx_group: KXGroup,
sig_type: SigType,
) {
// generate all harnesses (TlsBenchHarness structs) beforehand so that benchmarks
// generate all harnesses (TlsConnPair structs) beforehand so that benchmarks
// only include negotiation and not config/connection initialization
bench_group.bench_function(name, |b| {
bench_group.bench_function(T::name(), |b| {
b.iter_batched_ref(
|| {
T::new(
CryptoConfig::new(CipherSuite::default(), ec_group, sig_type),
TlsConnPair::<T, T>::new(
CryptoConfig::new(CipherSuite::default(), kx_group, sig_type),
handshake_type,
Default::default(),
)
},
|harness| {
|conn_pair_res| {
// harnesses with certain parameters fail to initialize for
// some past versions of s2n-tls, but missing data can be
// visually interpolated in the historical performance graph
if let Ok(harness) = harness {
let _ = harness.handshake();
if let Ok(conn_pair) = conn_pair_res {
let _ = conn_pair.handshake();
}
},
BatchSize::SmallInput,
)
});
}

for handshake_type in [ServerAuth, MutualAuth] {
for ec_group in [SECP256R1, X25519] {
for sig_type in [Rsa2048, Rsa3072, Rsa4096, Ec384] {
for handshake_type in HandshakeType::iter() {
for kx_group in KXGroup::iter() {
for sig_type in SigType::iter() {
let mut bench_group = c.benchmark_group(format!(
"handshake-{:?}-{:?}-{:?}",
handshake_type, ec_group, sig_type
handshake_type, kx_group, sig_type
));
bench_handshake_for_library::<S2NHarness>(
bench_handshake_for_library::<S2NConnection>(
&mut bench_group,
"s2n-tls",
handshake_type,
ec_group,
kx_group,
sig_type,
);
#[cfg(not(feature = "historical-perf"))]
{
bench_handshake_for_library::<RustlsHarness>(
bench_handshake_for_library::<RustlsConnection>(
&mut bench_group,
"rustls",
handshake_type,
ec_group,
kx_group,
sig_type,
);
bench_handshake_for_library::<OpenSslHarness>(
bench_handshake_for_library::<OpenSslConnection>(
&mut bench_group,
"openssl",
handshake_type,
ec_group,
kx_group,
sig_type,
);
}
Expand Down
32 changes: 14 additions & 18 deletions bindings/rust/bench/benches/throughput.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,29 @@
// SPDX-License-Identifier: Apache-2.0

use bench::{
CipherSuite::{self, *},
CryptoConfig, ECGroup, HandshakeType, OpenSslHarness, RustlsHarness, S2NHarness, SigType,
TlsBenchHarness, harness::ConnectedBuffer,
harness::ConnectedBuffer, CipherSuite, CryptoConfig, HandshakeType, KXGroup, OpenSslConnection,
RustlsConnection, S2NConnection, SigType, TlsConnPair, TlsConnection,
};
use criterion::{
criterion_group, criterion_main, measurement::WallTime, BatchSize, BenchmarkGroup, Criterion,
Throughput,
};
use strum::IntoEnumIterator;

pub fn bench_throughput_cipher_suite(c: &mut Criterion) {
// arbitrarily large to cut across TLS record boundaries
let mut shared_buf = [0u8; 100000];

fn bench_throughput_for_library<T: TlsBenchHarness>(
fn bench_throughput_for_library<T: TlsConnection>(
bench_group: &mut BenchmarkGroup<WallTime>,
name: &str,
shared_buf: &mut [u8],
cipher_suite: CipherSuite,
) {
bench_group.bench_function(name, |b| {
bench_group.bench_function(T::name(), |b| {
b.iter_batched_ref(
|| {
T::new(
CryptoConfig::new(cipher_suite, ECGroup::default(), SigType::default()),
TlsConnPair::<T, T>::new(
CryptoConfig::new(cipher_suite, KXGroup::default(), SigType::default()),
HandshakeType::default(),
ConnectedBuffer::default(),
)
Expand All @@ -34,36 +33,33 @@ pub fn bench_throughput_cipher_suite(c: &mut Criterion) {
h
})
},
|harness| {
if let Ok(harness) = harness {
let _ = harness.round_trip_transfer(shared_buf);
|conn_pair_res| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think I like conn_pair over conn_pair_res

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a Result, so I want to qualify that with the variable name to make it extra clear; if someone later tried to change this, the type of the variable would hopefully be one less thing to reason through.

if let Ok(conn_pair) = conn_pair_res {
let _ = conn_pair.round_trip_transfer(shared_buf);
}
},
BatchSize::SmallInput,
)
});
}

for cipher_suite in [AES_128_GCM_SHA256, AES_256_GCM_SHA384] {
for cipher_suite in CipherSuite::iter() {
let mut bench_group = c.benchmark_group(format!("throughput-{:?}", cipher_suite));
bench_group.throughput(Throughput::Bytes(shared_buf.len() as u64));
bench_throughput_for_library::<S2NHarness>(
bench_throughput_for_library::<S2NConnection>(
&mut bench_group,
"s2n-tls",
&mut shared_buf,
cipher_suite,
);
#[cfg(not(feature = "historical-perf"))]
{
bench_throughput_for_library::<RustlsHarness>(
bench_throughput_for_library::<RustlsConnection>(
&mut bench_group,
"rustls",
&mut shared_buf,
cipher_suite,
);
bench_throughput_for_library::<OpenSslHarness>(
bench_throughput_for_library::<OpenSslConnection>(
&mut bench_group,
"openssl",
&mut shared_buf,
cipher_suite,
);
Expand Down
15 changes: 8 additions & 7 deletions bindings/rust/bench/certs/generate_certs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cert-gen () {

key_family=$1
key_size=$2
dir_name=$3
maddeleine marked this conversation as resolved.
Show resolved Hide resolved

# set openssl argument name
if [[ $key_family == rsa ]]; then
Expand All @@ -30,8 +31,8 @@ cert-gen () {
fi

# make directory for certs
mkdir -p $key_family$key_size
cd $key_family$key_size
mkdir -p $dir_name
cd $dir_name

echo "generating CA private key and certificate"
openssl req -new -nodes -x509 -newkey $key_family -pkeyopt $argname$key_size -keyout ca-key.pem -out ca-cert.pem -days 65536 -config ../config/ca.cnf
Expand Down Expand Up @@ -62,13 +63,13 @@ cert-gen () {

if [[ $1 != "clean" ]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not in scope for this PR

I kind of want to slap myself for suggesting this, but would a make file make sense here? You could define the targets for the various certificate types, which would allow you to specify the other files that need to exist. And make clean is a well recognized idiom.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There other files that need to exist in order to generate the certs are all deleted afterwards anyways, while (to my understanding) a Makefile is especially useful if you don't need to recompile everything for a small code change, since the intermediate artifacts (like .o files) are cached. With the certs, there's not really a concept for the small code change; sure you can keep the private keys around to be able to regenerate and resign the certs, but there's no need for that for benching. Also, something that might be done in the future is to change how long the cert chain is, which feels more natural as a command line argument than as an option in a Makefile. Overall, Makefiles add more complexity than needed; hopefully the script is fairly readable as-is.

then
cert-gen ec 384
cert-gen rsa 2048
cert-gen rsa 3072
cert-gen rsa 4096
cert-gen ec 384 ecdsa384
maddeleine marked this conversation as resolved.
Show resolved Hide resolved
cert-gen rsa 2048 rsa2048
cert-gen rsa 3072 rsa3072
cert-gen rsa 4096 rsa4096
else
echo "cleaning certs"
rm -rf ec*/ rsa*/
rm -rf ecdsa*/ rsa*/
fi

popd > /dev/null
2 changes: 1 addition & 1 deletion bindings/rust/bench/src/bin/graph_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn get_bytes_from_snapshot(name: &str, i: i32) -> i32 {
}

/// Get the difference in bytes between two snapshots, which is memory of the
/// `i`th TlsBenchHarness (client and server)
/// `i`th TlsConnPair (client and server)
fn get_bytes_diff(name: &str, i: i32) -> i32 {
get_bytes_from_snapshot(name, i + 1) - get_bytes_from_snapshot(name, i)
}
Expand Down
33 changes: 18 additions & 15 deletions bindings/rust/bench/src/bin/memory.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

use bench::{harness::ConnectedBuffer, OpenSslHarness, RustlsHarness, S2NHarness, TlsBenchHarness};
use bench::{
harness::ConnectedBuffer, CryptoConfig, HandshakeType, OpenSslConnection, RustlsConnection,
S2NConnection, TlsConnPair, TlsConnection,
};
use std::{fs::create_dir_all, path::Path};

fn memory_bench<T: TlsBenchHarness>(dir_name: &str) {
fn memory_bench<T: TlsConnection>(dir_name: &str) {
println!("testing {dir_name}");

if !Path::new(&format!("target/memory/{dir_name}")).is_dir() {
create_dir_all(format!("target/memory/{dir_name}")).unwrap();
}

let mut harnesses = Vec::new();
harnesses.reserve(100);
let mut conn_pairs = Vec::new();
conn_pairs.reserve(100);

// reserve space for buffers before benching
let mut buffers = Vec::new();
Expand All @@ -22,27 +25,27 @@ fn memory_bench<T: TlsBenchHarness>(dir_name: &str) {
}

// handshake one harness to initalize libraries
let mut harness = T::default().unwrap();
harness.handshake().unwrap();
let mut conn_pair = TlsConnPair::<T, T>::default();
conn_pair.handshake().unwrap();

// tell massif to take initial memory snapshot
crabgrind::monitor_command(format!("snapshot target/memory/{dir_name}/0.snapshot")).unwrap();

// make and handshake 100 harness
// make and handshake 100 connection pairs
// memory usage stabilizes after first few handshakes
for i in 1..101 {
// put new harness directly into harness vec
harnesses.push(
T::new(
Default::default(),
Default::default(),
conn_pairs.push(
TlsConnPair::<T, T>::new(
CryptoConfig::default(),
HandshakeType::default(),
buffers.pop().unwrap(), // take ownership of buffer
)
.unwrap(),
);

// handshake last harness added
harnesses
conn_pairs
.as_mut_slice()
.last_mut()
.unwrap()
Expand All @@ -58,7 +61,7 @@ fn memory_bench<T: TlsBenchHarness>(dir_name: &str) {
fn main() {
assert!(!cfg!(debug_assertions), "need to run in release mode");

memory_bench::<S2NHarness>("s2n-tls");
memory_bench::<RustlsHarness>("rustls");
memory_bench::<OpenSslHarness>("openssl");
memory_bench::<S2NConnection>("s2n-tls");
memory_bench::<RustlsConnection>("rustls");
memory_bench::<OpenSslConnection>("openssl");
}
Loading
Loading