quiche: Add inline variant to ConnectionId#2460
Conversation
TheJokr
force-pushed
the
lblocher/inline-cid
branch
from
708d748 to
4e41f20
Compare
The existing `ConnectionId` struct is 24 bytes in size, which is sufficient to store any QUIC v1 connection ID inline. Taking advantage of this allows us to avoid allocations when storing and moving CIDs around throughout quiche and tokio-quiche. The implementation is heavily inspired by `CidOwned` in tokio-quiche, which I will replace in the next commit. This includes optimizations to make hashing and comparisons more efficient by interpreting the entire inline storage as a u64 array to enable fewer comparisons.
Now that ConnectionId has inline storage, `cid.to_vec().into()` is a needlessly inefficient way to clone it.
`ConnectionIdInline` needs Rust 1.87 for const `slice::split_at_mut_checked` in its `new()` method.
TheJokr
force-pushed
the
lblocher/inline-cid
branch
from
4e41f20 to
0119ddf
Compare
|
Nice. I would actually get rid of ConnectionIdInline enum and instead just a SmallVec that stores up to 20 bytes (if we want to support CID of unbounded length). If we just need up to 20bytes, we can just an inline storage. |
| struct ConnectionIdInline { | ||
| storage: [u8; 23], | ||
| len: CidInlineLen, | ||
| } |
There was a problem hiding this comment.
Using a 23 byte inline array means that ConnectionIdInline can't use niche optimization for enums, so e.g., an Option<ConnectionIdLine> (or ConnectionIdInner) will actually use 32 bytes. Instead, if you limit the inline storage it to 22 bytes, Option should take just 24 bytes. (You'll want to double check with the compiler though :-) )
There was a problem hiding this comment.
Ha, that's why we have CidInlineLen! CidInlineLen provides the niches, which is why ConnectionIdInner is only 24 bytes in total.
The same applies to Option<ConnectionIdInline> and even Option<ConnectionId>.
There was a problem hiding this comment.
Ah, yes. It would have helped if I read the all the changes :-)
Instead of the match, you can also use NonZeroU8. And encode it as len + 1. (i.e., non_zero_len==1 represents an actual length of 0). Implication is you can't represent a length of 255. (Or more generally create a NonMaxU8 struct that uses NonZero internally ¯_(ツ)_/¯
2 participants
I recommend reviewing this PR commit-by-commit.
The existing
ConnectionIdstruct is 24 bytes in size, which issufficient to store any QUIC v1 connection ID inline. Taking advantage
of this allows us to avoid allocations when storing and moving CIDs
around throughout quiche and tokio-quiche.
The implementation is heavily inspired by
CidOwnedin tokio-quiche,which this new implementation replaces. This includes optimizations to
make hashing and comparisons more efficient by interpreting the entire
inline storage as a u64 array to enable fewer comparisons.
The public API for
ConnectionIdstays exactly the same, except thatConnectionId::from_veccan't be
constanymore. I had to raise the MSRV to 1.87 to enable the restof the functions to stay
const.