perf fix for channel over MetaTls #636
Open
+47
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Differential Revision: D78831200
Summary: Throughput of channel on MetaTls is surprisingly low, it is much lower than regular TCP without TLS. Profiling shows most of the time is spent in memset of buffer: https://fburl.com/strobelight/bxxdyloh, Hot stack: P1879123816 Root cause: TLS record size is 16KB, so every poll will get 16KB of data, but we use a buf of total message size to receive the record, in my test, it is 70MB. The worst thing is, TLS lib will zero-out the whole 70MB every time: e.g. P1879474802. As a result, the larger the message, the worse the perf. This diff propose a mitigate to the problem, but it makes changes to rustls. * In monarch, we can get around of this problem but implementing our own framing, making sure size of each fame is fairly small. * But this is also a real problem in rust TLS lib, its API doesn't tell user about such a perf trap. It should either fix the perf issue, or explicitly asking users to always use small buffer (which I don't think it is desible behavior), so I still think it should be fixed in Rust TLS. Differential Revision: D78875272
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D78875272 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Throughput of channel on MetaTls is surprisingly low, it is much lower than regular TCP without TLS.
Profiling shows most of the time is spent in memset of buffer: https://fburl.com/strobelight/bxxdyloh,
Hot stack: P1879123816
Root cause:
TLS record size is 16KB, so every poll will get 16KB of data, but we use a buf of total message size to
receive the record, in my test, it is 70MB. The worst thing is, TLS lib will zero-out the whole 70MB
every time: e.g. P1879474802. As a result, the larger the message, the worse the perf.
This diff propose a mitigate to the problem, but it makes changes to rustls.
each fame is fairly small.
either fix the perf issue, or explicitly asking users to always use small buffer (which I don't think it
is desible behavior), so I still think it should be fixed in Rust TLS.
Differential Revision: D78875272