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

Various bulk benchmark improvements #1193

Merged
merged 4 commits into from
Sep 20, 2021
Merged

Various bulk benchmark improvements #1193

merged 4 commits into from
Sep 20, 2021

Conversation

Matthias247
Copy link
Contributor

This adds a set of improvements to the bulk benchmark:

  • Data transfer in the benchmark is now possible from client to server (existed before) as well as client to server (new). The latter was set as default, because its the more common use-case.
  • Data sizes can be specified with units (e.g. 10G)
  • Multiple clients can be run concurrently, which provides information how well endpoints scale in case multiple connections are used (they don't do well - throughput drops 550MB/s from to 350MB/s overall when going from 1 to 2 clients)

The first commit in the set refactors the executable a bit in order to avoid having a single huge file and should make the other changes a bit easier to follow.

Here's a invocation which gives a summary about the new functionality:

cargo run --release --bin bulk --  --clients 2 --download-size 1G --upload-size 1G

Client 0 stats:
Overall upload stats:

Transferred 1073741824 bytes on 1 streams in 10.54s (97.16 MiB/s)

Stream upload metrics:

      │  Throughput   │ Duration
──────┼───────────────┼──────────
 AVG  │  222.94 MiB/s │     4.59s
 P0   │  222.88 MiB/s │     4.59s
 P10  │  223.00 MiB/s │     4.60s
 P50  │  223.00 MiB/s │     4.60s
 P90  │  223.00 MiB/s │     4.60s
 P100 │  223.00 MiB/s │     4.60s
Overall download stats:

Transferred 1073741824 bytes on 1 streams in 10.54s (97.16 MiB/s)

Stream download metrics:

      │  Throughput   │ Duration
──────┼───────────────┼──────────
 AVG  │  172.19 MiB/s │     5.95s
 P0   │  172.12 MiB/s │     5.94s
 P10  │  172.25 MiB/s │     5.95s
 P50  │  172.25 MiB/s │     5.95s
 P90  │  172.25 MiB/s │     5.95s
 P100 │  172.25 MiB/s │     5.95s

Client 1 stats:
Overall upload stats:

Transferred 1073741824 bytes on 1 streams in 10.59s (96.65 MiB/s)

Stream upload metrics:

      │  Throughput   │ Duration
──────┼───────────────┼──────────
 AVG  │  210.69 MiB/s │     4.86s
 P0   │  210.62 MiB/s │     4.86s
 P10  │  210.75 MiB/s │     4.86s
 P50  │  210.75 MiB/s │     4.86s
 P90  │  210.75 MiB/s │     4.86s
 P100 │  210.75 MiB/s │     4.86s
Overall download stats:

Transferred 1073741824 bytes on 1 streams in 10.59s (96.65 MiB/s)

Stream download metrics:

      │  Throughput   │ Duration
──────┼───────────────┼──────────
 AVG  │  178.56 MiB/s │     5.73s
 P0   │  178.50 MiB/s │     5.73s
 P10  │  178.62 MiB/s │     5.74s
 P50  │  178.62 MiB/s │     5.74s
 P90  │  178.62 MiB/s │     5.74s
 P100 │  178.62 MiB/s │     5.74s

@Matthias247 Matthias247 force-pushed the bench branch 2 times, most recently from 26c83b6 to 803f2af Compare September 20, 2021 02:53
In order to extend the bulk benchmark further, this change moves most
common functions out of the bulk.rs binary and into a lib.rs and stats.rs file.

Functionally there are no relevant changes.
This changes the bulk benchmark to support server->client
in addition to client->server transfers.

The default direction is set to server->client, since
this is more typically used.
@djc
Copy link
Member

djc commented Sep 20, 2021

Nice work!

Comment on lines 63 to 64
drain_stream(&mut recv_stream, opt.read_unordered).await?;
send_data_on_stream(&mut send_stream, opt.download_size).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these be concurrent? Sequential seems of limited value, since you could get similar data from just running two tests. Alternatively, use unidirectional streams in both directions, with handlers spawned independently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it. But that would require adding either a join! which adds overhead due to polling non-ready tasks, or adding more subtasks or futures-unordered. In the end I decided its not worth it since testing both directions at the same time doesn't seem super useful, and sequential still gives useful values since at least the per-stream metrics only cover the actual transfer time inside the stream.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we use unidirectional streams in each direction, I don't think the actual number of subtasks needs to change, since streams for an unused transfer direction can be dynamically omitted.

That said, I don't see a clear requirement for the capability, so 🤷, nonblocking.

This makes it a bit easier to specify large arguments
This allows to specify a "--client" parameter in the benchmark,
which specifies the amount of clients which perform a  test concurrently.
Each client uses its own thread and single-threaded runtime.

Stats are currently printed for each individual client and not aggregated.
@djc djc merged commit 8899b1a into quinn-rs:main Sep 20, 2021
@Matthias247 Matthias247 deleted the bench branch September 21, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants