RUST-1337 Use tokio's AsyncRead and AsyncWrite traits
#668
Conversation
|
Out of curiosity, is there a way to see how this patch affects the existing perf graph you linked from the jira ticket? or did you just confirm the perf has improved manually? |
|
So if you click on a task (e.g. test-5.0-standalone) and click on a data point (a little arrow should appear), the % difference column will be populated. So for example, on the pre-regression point for BenchRead I'm seeing anywhere from -5% in score to +5%, which seems to be on track. |
Co-authored-by: Abraham Egnor <[email protected]>
| }) | ||
| let conn = connector | ||
| .connect_with(name, tcp_stream, |c| { | ||
| c.set_buffer_limit(None); |
There was a problem hiding this comment.
why do we need to set this explicitly now?
There was a problem hiding this comment.
I'm not actually sure if this is required or not, but in the latest rustls version the default limit was changed from unlimited (what we use here) to 64KB. I changed it back to unlimited as that was my first guess at why the perf regression happened, though it alone didn't seem to have a significant impact. That said, rustls sets it to None in their benchmarks, so perhaps its worth keeping here.
There was a problem hiding this comment.
sgtm to explicitly set it to future-proof things
This fixes a performance regression that resulted from upgrading tokio-rustls and/or rustls.
RUST-1337
Upgrading to the most recent version
tokio-rustlsand/orrustlscaused a significant performance reduction in the driver when usingrustls. This seems to have been fixed by switching to using tokio'sAsyncReadtrait for our stream types. I'm not entirely sure why this is the case to be honest, but it seems to work and reduced the code complexity a bit 🤷. For consistency, I also updated the write ends to use tokio'sAsyncWrite, though that didn't have any effect on performance. As part of this, we can now remove the dependency onfutures_io.perf patch: https://evergreen.mongodb.com/version/6296cd21d6d80a79a5061a14