Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Tracking issue for cleaning-up allocations #1733

Open
11 tasks
timotree3 opened this issue Oct 3, 2019 · 4 comments
Open
11 tasks

Tracking issue for cleaning-up allocations #1733

timotree3 opened this issue Oct 3, 2019 · 4 comments

Comments

@timotree3
Copy link
Collaborator

timotree3 commented Oct 3, 2019

The current code-base does a lot of unnecessary allocations. Here are some examples of common things in the code-base that cause allocation:

  • format!
  • .to_string()
  • .clone()
  • .collect()
  • .to_path_buf()
  • .to_string_lossy()
  • .into_vec()

This issue is intended to classify the different patterns in the code-base that allocate unnecessarily and track the progress in cleaning them up.

  • Calling functions that take &Strings (in progress: String roundup part 1 #1732)
  • Calling functions that take &PathBuf (in progress: Path string roundup #1701)
  • Converting between strings and paths (in progress: Path string roundup #1701)
  • Calling format! only to use the result as a format argument
  • Taking things by reference just to immediately clone them
  • Allocating PathBufs only to join them (in progress: Allocate fewer PathBufs only to join them #1736)
  • Calling .collect() only to iterate over the result
  • Calling .to_string() only to use the result for comparison
  • Converting Strings to JsonStrings
  • Returning String errors
  • Storing owned collections in a struct when giving the struct a lifetime and storing references would do

Feel free to check these boxes or add more to-dos if any come up! I believe maintainers are able to edit this post.

@timotree3
Copy link
Collaborator Author

As I've started to do with these two "roundup" PRs. I intend to slowly check off each box on this list.

@thedavidmeister
Copy link
Contributor

sounds good :)

my request would be to keep PRs as easy to review and focussed as possible

@timotree3
Copy link
Collaborator Author

I'm totally with you! :-) I've been making a serious effort to do just that.

I've been going for the strategy of each commit having one well explained transformation, applied across the code base. The only other option that I've thought of is to apply all the transformations to a single module and then make a PR for each module.

Is there some approach that you have in mind?

@thedavidmeister
Copy link
Contributor

@timotree3 nothing specific, just keep it as easy to review as possible :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants