Skip to content
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

codewide: small performance optimisations #1169

Merged
merged 6 commits into from
Jan 16, 2025

Conversation

wprzytula
Copy link
Collaborator

This is a bunch of tiny optimisations around reducing number of allocations.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • [ ] I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have provided docstrings for the public items that I want to introduce.
  • [ ] I have adjusted the documentation in ./docs/source/.
  • [ ] I added appropriate Fixes: annotations to PR description.

In multiple places in the code, `format!` is called only to subsequently
pass the allocated String to a printing macro.
In all such places, `format!` could be trivially elided.
@wprzytula wprzytula requested a review from muzarski January 15, 2025 19:57
@wprzytula wprzytula self-assigned this Jan 15, 2025
Copy link

github-actions bot commented Jan 15, 2025

cargo semver-checks found no API-breaking changes in this PR! 🎉🥳
Checked commit: a42992f

Instead of allocating a `Vec<_>`, one can leverage
`CommaSeparatedDisplayer` to print a series of items in the
comma-separated manner.
RequestSpan::record_replicas now:
1. accepts an iterator, not necessarily a slice,
2. utilises CommaSeparatedDisplayer instead of hand-crafted comma
  management.

In the next commit, the point 1. is taken advantage of.
Even though the replicas were allocated in a SmallVec, with the enhanced
RequestSpan::record_replicas implementation we can elide the call to
`collect()`.
In order to make this possible, several iterators in the replica locator
have now `Clone` derived.
Perhaps the compiler would optimise this anyway, but now it's explicitly
better.
@wprzytula wprzytula force-pushed the small-optimisations branch from 7225d4b to a42992f Compare January 16, 2025 08:44
@wprzytula wprzytula merged commit f9f0940 into scylladb:main Jan 16, 2025
11 checks passed
@wprzytula wprzytula deleted the small-optimisations branch January 16, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants