perf(value): back KeyString with SmolStr for zero-alloc inline key storage#1825
Open
thomasqueirozb wants to merge 6 commits into
Open
perf(value): back KeyString with SmolStr for zero-alloc inline key storage#1825thomasqueirozb wants to merge 6 commits into
thomasqueirozb wants to merge 6 commits into
Conversation
SmolStr doesn't implement proptest::arbitrary::Arbitrary, so the proptest_derive macro fails. Replace the derive with a manual impl that maps any::<String>() through KeyString::from. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
9 tasks
pront
previously approved these changes
Jun 17, 2026
pront
left a comment
pront
left a comment
Member
There was a problem hiding this comment.
Nice. I shared some concerns offline, so let's wait until we have an SMP and results are well understood before merging.
`BorrowedSegment::Field` wraps a `Cow<'_, str>` which is always `Borrowed` when constructed from the `path!()` macro. Previously `field.to_string().into()` allocated a transient `String` only to immediately copy it into a `KeyString` and drop it. With SmolStr-backed `KeyString`, `From<Cow<'_, str>>` goes directly to `SmolStr::new` — inline for keys ≤22 bytes, no heap allocation involved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
|
Did you look into any of the other "small string" crates like compact_str or smartstring? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
KeyStringis now backed bySmolStrinstead ofString. Strings up to 22 bytes are stored inline in the struct (no heap allocation). All common event key names (regex capture group names, log field names likemessage/host/timestamp) fit within that limit, soclone()becomes a plain 24-byte stack copy instead of amalloc+memcpy.Benchmarked on two Vector test harness workloads (5x c5.large producers -> 1x c5.large subject -> consumer, 60s measurement window):
Change Type
Is this a breaking change?
How did you test this PR?
Ran the Vector regex_parsing_performance and real_world_1_performance integration benchmarks (5x c5.large producers -> 1x c5.large Vector -> consumer, 60s measurement window). Confirmed all existing VRL unit tests pass.
Does this PR include user facing changes?
Checklist
dd-rust-license-tool writeand commit the changes. More details here.References
NA