-
Notifications
You must be signed in to change notification settings - Fork 780
Make ciphersuite enum smaller #2172
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
Conversation
Use `#[repr(u16)]` to define wire type, rather than `@U16`.
This changes the output for `Unknown` variants: - from `Unknown(<decimal value>)` - to `TypeName(0x<hex value>)` in preparation for eliding other uncommon variants in Debug output. The hex output matches how IANA documents the most common ones: cipher suites and signature schemes.
Benchmark resultsInstruction countsSignificant differencesThere are no significant instruction count differences Other differencesClick to expand
Wall-timeSignificant differencesThere are no significant wall-time differences Other differencesClick to expand
Additional informationCheckout details:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2172 +/- ##
==========================================
- Coverage 94.71% 94.71% -0.01%
==========================================
Files 102 102
Lines 23748 23769 +21
==========================================
+ Hits 22493 22512 +19
- Misses 1255 1257 +2 ☔ View full report in Codecov by Sentry. |
cpu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I concur with @djc that the nodebug syntax in the macro feels a little bit unusual. Not a deal breaker ofc.
17abb94 to
b5ab4cc
Compare
Arrange that uncommon variants have their match arms in `Debug::fmt` omitted. Things are uncommon if they are prior to TLS1.2, or were never commonly used in TLS1.2 -- it is not a 100% overlap with things we implement. Before: > 0.1% 0.4% 9.0KiB rustls <rustls::enums::CipherSuite as core::fmt::Debug>::fmt After: > 0.0% 0.0% 768B rustls <rustls::enums::CipherSuite as core::fmt::Debug>::fmt
b5ab4cc to
f67992e
Compare
cpu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New syntax looks like an improvement to me, thanks 👍
We can't remove variants without a breaking API change (saved #2171 for later) but we can reduce the expense and size of
Debugon it now.This doesn't do anything for
fn to_strbut that function doesn't have the viral property thatDebughas: if you don't want the strings in that function, just don't call it.fixes #2138