( )⚙ D7299 rust-status: return a ParallelIterator instead of a Vec from stat_dmap_entries

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7299

rust-status: return a ParallelIterator instead of a Vec from stat_dmap_entries
ClosedPublic

Authored by Alphare on Nov 7 2019, 5:18 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG75fe6e71ddb8: rust-status: return a ParallelIterator instead of a Vec from stat_dmap_entries
Summary

This allows the caller function to choose when and how the iteration and/or
collection happens. This change also cleans up the now unused filter_map.

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

Alphare created this revision.Nov 7 2019, 5:18 AM
kevincox accepted this revision.Nov 7 2019, 1:22 PM
kevincox added inline comments.
rust/hg-core/src/dirstate/status.rs
126

This can be replaced with:

let meta = root_dir.as_ref().join(hg_path_to_path_buf(filename)?).symlink_metadata();

Although I would probably also put hg_path_to_path_buf(filename)? into a variable for readability.

Alphare updated this revision to Diff 17740.Nov 8 2019, 7:51 AM
Alphare marked an inline comment as done.Nov 8 2019, 7:54 AM
Alphare added a commit: rHG75fe6e71ddb8: rust-status: return a ParallelIterator instead of a Vec from stat_dmap_entries.Nov 11 2019, 4:08 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG75fe6e71ddb8: rust-status: return a ParallelIterator instead of a Vec from stat_dmap_entries (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/dirstate/status.rs (90 lines)

Diff 18018

rust/hg-core/src/dirstate/status.rs

EntryState::Unknown => Dispatch::Unknown, EntryState::Unknown => Dispatch::Unknown,
} }
} }
/// Stat all entries in the `DirstateMap` and mark them for dispatch into /// Stat all entries in the `DirstateMap` and mark them for dispatch into
/// the relevant collections. /// the relevant collections.
fn stat_dmap_entries( fn stat_dmap_entries(
dmap: &DirstateMap, dmap: &DirstateMap,
root_dir: impl AsRef<Path> + Sync, root_dir: impl AsRef<Path> + Sync + Send,
check_exec: bool, check_exec: bool,
list_clean: bool, list_clean: bool,
last_normal_time: i64, last_normal_time: i64,
) -> std::io::Result<Vec<(&HgPath, Dispatch)>> { ) -> impl ParallelIterator<Item = std::io::Result<(&HgPath, Dispatch)>> {
dmap.par_iter() dmap.par_iter().map(move |(filename, entry)| {
.filter_map( let filename: &HgPath = filename;
// Getting file metadata is costly, so we don't do it if the let filename_as_path = hg_path_to_path_buf(filename)?;
// file is already present in the results, hence `filter_map` let meta = root_dir.as_ref().join(filename_as_path).symlink_metadata();
|(filename, entry)| -> Option<
std::io::Result<(&HgPath, Dispatch)>
> {
let meta = match hg_path_to_path_buf(filename) {
Ok(p) => root_dir.as_ref().join(p).symlink_metadata(),
Err(e) => return Some(Err(e.into())),
};
Some(match meta { match meta {
kevincoxUnsubmitted
Done

This can be replaced with:

let meta = root_dir.as_ref().join(hg_path_to_path_buf(filename)?).symlink_metadata();

Although I would probably also put hg_path_to_path_buf(filename)? into a variable for readability.

kevincox: This can be replaced with: ``` let meta = root_dir.as_ref().join(hg_path_to_path_buf(filename)?
Ok(ref m) Ok(ref m)
if !(m.file_type().is_file() if !(m.file_type().is_file()
|| m.file_type().is_symlink()) => || m.file_type().is_symlink()) =>
{ {
Ok((filename, dispatch_missing(entry.state))) Ok((filename, dispatch_missing(entry.state)))
} }
Ok(m) => Ok((filename, dispatch_found( Ok(m) => Ok((
filename,
dispatch_found(
filename, filename,
*entry, *entry,
HgMetadata::from_metadata(m), HgMetadata::from_metadata(m),
&dmap.copy_map, &dmap.copy_map,
check_exec, check_exec,
list_clean, list_clean,
last_normal_time))), last_normal_time,
),
)),
Err(ref e) Err(ref e)
if e.kind() == std::io::ErrorKind::NotFound if e.kind() == std::io::ErrorKind::NotFound
|| e.raw_os_error() == Some(20) => || e.raw_os_error() == Some(20) =>
{ {
// Rust does not yet have an `ErrorKind` for // Rust does not yet have an `ErrorKind` for
// `NotADirectory` (errno 20) // `NotADirectory` (errno 20)
// It happens if the dirstate contains `foo/bar` and // It happens if the dirstate contains `foo/bar` and
// foo is not a directory // foo is not a directory
Ok((filename, dispatch_missing(entry.state))) Ok((filename, dispatch_missing(entry.state)))
} }
Err(e) => Err(e), Err(e) => Err(e),
}
}) })
},
)
.collect()
} }
pub struct StatusResult<'a> { pub struct StatusResult<'a> {
pub modified: Vec<&'a HgPath>, pub modified: Vec<&'a HgPath>,
pub added: Vec<&'a HgPath>, pub added: Vec<&'a HgPath>,
pub removed: Vec<&'a HgPath>, pub removed: Vec<&'a HgPath>,
pub deleted: Vec<&'a HgPath>, pub deleted: Vec<&'a HgPath>,
pub clean: Vec<&'a HgPath>, pub clean: Vec<&'a HgPath>,
deleted, deleted,
clean, clean,
}, },
) )
} }
pub fn status( pub fn status(
dmap: &DirstateMap, dmap: &DirstateMap,
root_dir: impl AsRef<Path> + Sync + Copy, root_dir: impl AsRef<Path> + Sync + Send + Copy,
list_clean: bool, list_clean: bool,
last_normal_time: i64, last_normal_time: i64,
check_exec: bool, check_exec: bool,
) -> std::io::Result<(Vec<&HgPath>, StatusResult)> { ) -> std::io::Result<(Vec<&HgPath>, StatusResult)> {
let results = stat_dmap_entries( let results: std::io::Result<_> = stat_dmap_entries(
&dmap, &dmap,
root_dir, root_dir,
check_exec, check_exec,
list_clean, list_clean,
last_normal_time, last_normal_time,
)?; )
.collect();
Ok(build_response(results)) Ok(build_response(results?))
} }