This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10199

rust: Preallocate the returned `Vec` in `utils::files::relativize_path`
ClosedPublic

Authored by SimonSapin on Mar 13 2021, 3:01 AM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHGc94fa884240b: rust: Preallocate the returned `Vec` in `utils::files::relativize_path`
Summary

Profiling rhg files > /dev/null on an old snapshot of mozilla-central
(with perf and the Firefox Profiler:
https://github.com/firefox-devtools/profiler/blob/main/docs-user/guide-perf-profiling.md)
showed non-trivial time spend in this function and in realloc.

This change makes the wall-clock time for that process on my machine
go from ~190 ms to ~150 ms.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Mar 13 2021, 3:01 AM
SimonSapin edited the summary of this revision. (Show Details)Mar 13 2021, 3:08 AM
Alphare accepted this revision.Mar 15 2021, 7:28 AM
Alphare added a subscriber: Alphare.

This will potentially cost a little bit more memory, but it's absolutely worth it for the speed gains.

SimonSapin added a comment.Mar 15 2021, 7:34 AM

I haven’t measured memory at all but I’d expect the effect to be very small or not noticeable since this function is typically called for printing the return value to stdout then deallocating it immediately. We’re not accumulating many "relativized" paths in memory at any given time.

pulkit accepted this revision.Mar 17 2021, 9:50 AM
This revision is now accepted and ready to land.Mar 17 2021, 9:50 AM

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/utils/files.rs (8 lines)

Diff 26483

rust/hg-core/src/utils/files.rs

/// ///
/// let cwd = HgPath::new(b"other"); /// let cwd = HgPath::new(b"other");
/// assert_eq!(relativize_path(file, cwd), Cow::Borrowed(b"../nested/file")); /// assert_eq!(relativize_path(file, cwd), Cow::Borrowed(b"../nested/file"));
/// ``` /// ```
pub fn relativize_path(path: &HgPath, cwd: impl AsRef<HgPath>) -> Cow<[u8]> { pub fn relativize_path(path: &HgPath, cwd: impl AsRef<HgPath>) -> Cow<[u8]> {
if cwd.as_ref().is_empty() { if cwd.as_ref().is_empty() {
Cow::Borrowed(path.as_bytes()) Cow::Borrowed(path.as_bytes())
} else { } else {
let mut res: Vec<u8> = Vec::new(); // This is not all accurate as to how large `res` will actually be, but
// profiling `rhg files` on a large-ish repo shows it’s better than
// starting from a zero-capacity `Vec` and letting `extend` reallocate
// repeatedly.
let guesstimate = path.as_bytes().len();
let mut res: Vec<u8> = Vec::with_capacity(guesstimate);
let mut path_iter = path.as_bytes().split(|b| *b == b'/').peekable(); let mut path_iter = path.as_bytes().split(|b| *b == b'/').peekable();
let mut cwd_iter = let mut cwd_iter =
cwd.as_ref().as_bytes().split(|b| *b == b'/').peekable(); cwd.as_ref().as_bytes().split(|b| *b == b'/').peekable();
loop { loop {
match (path_iter.peek(), cwd_iter.peek()) { match (path_iter.peek(), cwd_iter.peek()) {
(Some(a), Some(b)) if a == b => (), (Some(a), Some(b)) if a == b => (),
_ => break, _ => break,
} }