x509: reduce allocations for wrap_in_sequence #1563
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1563 +/- ##
=======================================
Coverage 96.41% 96.41%
=======================================
Files 75 75
Lines 15524 15525 +1
=======================================
+ Hits 14968 14969 +1
Misses 556 556
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
There was a problem hiding this comment.
Nice! Looks good to me.
Since this was a fix for a perf related issue it would be interesting to know if a tool like https://github.com/KDE/heaptrack shows the expected improvement under profiling.
Instead of taking a `Vec<u8>` and inserting bytes at the beginning, take a `&[u8]` and return a new vector containing those bytes plus a tag and a length. This isn't the perfect approach for all situations, but for one of the main places we call wrap_in_sequence (DistinguishedName::in_sequence), it's optimal because the input is `&[u8]`, meaning we can't write to a previously existing `Vec<u8>` (which would potentially save allocations by using excess capacity at the end of the Vec). In the process, change the one call site for `wrap_in_asn1_len` to call the new `asn1_wrap` function instead, which encodes a tag and length at the same time, reducing reallocations and copies. This has a slight secondary benefit: the resulting Vec is exactly sized to what it holds, instead of following the doubling approach and possibly over-allocating. This saves a handful of bytes in a long-lived data structure.
|
Pushed fixes.
I can take a look at this. Might take me a minute to get heaptrack set up. |
|
On main: On this branch: |
|
This feels small enough in scope that we can merge it without waiting on Ctz to be back. |
Instead of taking a
Vec<u8>and inserting bytes at the beginning, take a&[u8]and return a new vector containing those bytes plus a tag and a length.This isn't the perfect approach for all situations, but for one of the main places we call wrap_in_sequence (DistinguishedName::in_sequence), it's optimal because the input is
&[u8], meaning we can't write to a previously existingVec<u8>(which would potentially save allocations by using excess capacity at the end of the Vec).In the process, change the one call site for
wrap_in_asn1_lento call the newasn1_wrapfunction instead, which encodes a tag and length at the same time, reducing reallocations and copies.This has a slight secondary benefit: the resulting Vec is exactly sized to what it holds, instead of following the doubling approach and possibly over-allocating. This saves a handful of bytes in a long-lived data structure.
Fixes #1562