Skip to content

Cleanups - #6857

Merged
blp merged 3 commits into
mainfrom
cleanups
Aug 14, 2026
Merged

Cleanups#6857
blp merged 3 commits into
mainfrom
cleanups

Conversation

@blp

@blp blp commented Aug 13, 2026

Copy link
Copy Markdown
Member

These are some minor cleanups and improvements that I've been sitting on for a while. See individual commit messages for details.

blp added 3 commits August 13, 2026 13:52
I really cannot decipher the default printed form of VecIndexedWSet.  This
is better.

Signed-off-by: Ben Pfaff <blp@feldera.com>
Signed-off-by: Ben Pfaff <blp@feldera.com>
cur_key held a factory-allocated Box<DynOpt<T::Key>>, cloned from the
current key via from_ref() on every key transition. The clone is
unnecessary: the key reference comes from `updates`, a parameter
borrowed for the entire function call, so it stays valid across the
whole loop (and the final flush after it). Builder::push_key also only
ever takes a borrowed &Output::Key, never ownership. Hold a plain
Option<&T::Key> instead, dropping one allocation and copy per unique
key.

We can't do the same thing for cur_val because it gets mutated in-place
and even if we introduce some kind of Cow-like logic it ends up getting
copied most of the time anyway.

No measured macrobenchmark performance difference but it seems like an
obvious improvement.

Signed-off-by: Ben Pfaff <blp@feldera.com>
@blp
blp requested a review from ryzhyk August 13, 2026 21:01
@blp blp self-assigned this Aug 13, 2026
@blp blp added the rust Pull requests that update Rust code label Aug 13, 2026
@blp
blp marked this pull request as ready for review August 13, 2026 21:01
@blp
blp added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 6800e6b Aug 14, 2026
1 check passed
@blp
blp deleted the cleanups branch August 14, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants