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

Reuse existing small allocations if possible #12335

Merged
merged 4 commits into from Mar 30, 2024

Conversation

sholderbach
Copy link
Member

Those allocations are all small and insignificant in the grand scheme of things and the optimizer may be able to resolve some of those but better to be nice anyways.

Primarily inspired by the new clippy::assigning_clones

  • Avoid reallocs with clone_from in nu-parser
  • Avoid realloc on assignment in Stack
  • Fix clippy::assigning_clones in nu-cli
  • Reuse allocations in nu-explore if possible

- Associated clippy lint
https://rust-lang.github.io/rust-clippy/master/index.html#/clone_from
- This can save some allocations or drops by explicitly optimizing here.
- `Clone::clone_from` should provide pretty readable semantics
- `ToOwned::clone_into` maybe requires a bit more familiarity
- `String::clear(&mut self)` instead of assigning `String::new()`
- `Clone::clone_from` instead of assigning new allocations
- https://rust-lang.github.io/rust-clippy/master/index.html#/clone_from

Still some places that may be better expressed by e.g. `std::mem::take`
when passing around `String`s, but that could also be revisited with
good use of references
@kubouch
Copy link
Contributor

kubouch commented Mar 30, 2024

TIL clone_from() 💯

@sholderbach sholderbach merged commit cc39069 into nushell:main Mar 30, 2024
15 checks passed
@sholderbach sholderbach deleted the clippy-fun branch March 30, 2024 13:04
@hustcer hustcer added this to the v0.92.0 milestone Mar 30, 2024
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.

None yet

3 participants