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

Implement Extend::extend_reserve for faster string appending when final size is known #42

Open
kleinesfilmroellchen opened this issue Apr 25, 2023 · 0 comments · May be fixed by #43
Open

Comments

@kleinesfilmroellchen
Copy link

In spcasm's file handling code, I retrieve text from a file, strip out some characters with an iterator, and then collect the characters into a SmartString. The relevant snippet of code looks like this:

// type correctly inferred to SmartString<LazyCompact> due to other code
let contents = std::fs::read_to_string(&path)?.chars().filter(|c| c != &'\r').collect();

However, since I'm only removing some characters from pre-loaded text, I know an upper bound of text length. If I were able to tell SmartString to preallocate that amount of storage, it would most likely not need to repeatedly reallocate the string during appending from the iterator. Therefore, I would like to rewrite the above code as the following:

let text = std::fs::read_to_string(&path)?;
let text_size = text.len();
let mut contents = String::new();
<SmartString<LazyCompact> as Extend<char>>::extend_reserve(&mut contents, text_size); // <-----
contents.extend(text.chars().filter(|c| c != &'\r'));

This utilizes the unstable extend_one feature which provides extend_reserve. I would be fine with a separate function not in the trait while the feature is unstable since I don't need to access the function through the trait.

All other rewrites of the first snippet, especially the method of iteration, would fundamentally have the same issue and also benefit from this feature.

For a point of reference, the first code snippet is responsible for >16% of runtime on some of my inputs.

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 a pull request may close this issue.

1 participant