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

Reduce allocations in fs::copy_files_except_ext #2355

Merged

Conversation

johamster
Copy link
Contributor

Above mentioned function copies files (recursively) from a source to a destination directory. For that, file/directory paths have to be created repeatedly. This allocates as directory and file names are concatenated into an owning path structure.

The number of allocations can be reduced by creating file/directory paths only once and borrowing them instead of cloning/recreating them.

In bigger projects, this reduces execution time noticeably. Please note that file system operations are dominant from performance POV.

Above mentioned function copies files (recursively) from a source to a
destination directory. For that, file/directory paths have to be created
repeatedly. This allocates as directory and file names are concatenated
into an owning path structure.

The number of allocations can be reduced by creating file/directory
paths only once and borrowing them instead of cloning/recreating them.

In bigger projects, this reduces execution time noticeably. Please note
that file system operations are dominant from performance POV.
} else {
fs::remove_file(item)?;
}
for item in fs::read_dir(dir)?.flatten() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, this is off-topic and just resolves a clippy warning.

if ext_blacklist.contains(&ext.to_str().unwrap()) {
continue;
}
}
debug!(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this trace as it did not really add value, in my opinion. The target file path is traced also below.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ehuss ehuss added this pull request to the merge queue Apr 8, 2024
Merged via the queue into rust-lang:master with commit 8ae86d4 Apr 8, 2024
8 checks passed
@johamster johamster deleted the reduce_allocations_when_copying_files branch April 9, 2024 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants