Fix throughput bench issues and add documentation #4130
Conversation
bindings/rust/bench/README.md
Outdated
| @@ -32,6 +32,8 @@ To bench with AWS-LC, Amazon's custom libcrypto implementation, first run `scrip | |||
|
|
|||
| The benchmarks can be run with the `cargo bench` command. Criterion will auto-generate an HTML report in `target/criterion/`. | |||
|
|
|||
| Throughput benchmarks measure round-trip throughput with the client and server connections in the same thread for symmetry. In practice, throughput for a single connection would be ~4x higher than the values from the benchmarks. | |||
There was a problem hiding this comment.
I don't think I totally understand this statement? Wouldn't it just be 2x higher?
I'm also wary of telling people what their numbers "might" look like. In fact, it might be a good idea to have a general disclaimer about how we abstract away a lot of the complexity that exists in real systems(network IO) so customers should always run real benchmarks themselves, etc.
There was a problem hiding this comment.
Throughput being one-way vs round-trip makes it 2x higher, and having send/recv be in the same thread instead of two different threads makes it 2x higher on top of that, multiplying to 4x.
Putting a general disclaimer in an obvious place seems to be a good idea; I'll add it at the top.
bindings/rust/bench/README.md
Outdated
| @@ -32,6 +32,8 @@ To bench with AWS-LC, Amazon's custom libcrypto implementation, first run `scrip | |||
|
|
|||
| The benchmarks can be run with the `cargo bench` command. Criterion will auto-generate an HTML report in `target/criterion/`. | |||
|
|
|||
| Throughput benchmarks measure round-trip throughput with the client and server connections in the same thread for symmetry. In practice, throughput for a single connection would be ~4x higher than the values from the benchmarks. | |||
There was a problem hiding this comment.
I would probably say up to ~4x, since it depends on if the sender and receiver are split into multiple threads or not. There's also factors like RTT, buffer sizes, CPU asymmetry, and link rates that will limit it as well.
There was a problem hiding this comment.
Changed to "could theoretically be up to ~4x higher"
Description of changes:
s2n-tls's
send()andrecv()in the benches currently ignore partial reads and writes; this PR adds functionality to properly handle partial reads/writes.Also, this PR adds more documentation for throughput benchmarks and more clearly calls out how throughput benches are round-trip and single-threaded.
Testing:
Results from benching don't seem to have changed. To test, run all scripts and benchmarks.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.