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

login
register
mail settings
Submitter phabricator
Date Nov. 7, 2019, 10:18 a.m.
Message ID <differential-rev-PHID-DREV-3aijb7u53qsy2qumewcl-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42894/
State Superseded
Headers show

Comments

phabricator - Nov. 7, 2019, 10:18 a.m.
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION 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`.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7299

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

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
phabricator - Nov. 7, 2019, 6:22 p.m.
kevincox added inline comments.
kevincox accepted this revision.

INLINE COMMENTS

> status.rs:128
> +            Err(e) => return Err(e.into()),
> +        };
>  

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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7299/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7299

To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate/status.rs b/rust/hg-core/src/dirstate/status.rs
--- a/rust/hg-core/src/dirstate/status.rs
+++ b/rust/hg-core/src/dirstate/status.rs
@@ -115,53 +115,50 @@ 
 /// the relevant collections.
 fn stat_dmap_entries(
     dmap: &DirstateMap,
-    root_dir: impl AsRef<Path> + Sync,
+    root_dir: impl AsRef<Path> + Sync + Send,
     check_exec: bool,
     list_clean: bool,
     last_normal_time: i64,
-) -> std::io::Result<Vec<(&HgPath, Dispatch)>> {
-    dmap.par_iter()
-        .filter_map(
-            // Getting file metadata is costly, so we don't do it if the
-            // file is already present in the results, hence `filter_map`
-            |(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())),
-                };
+) -> impl ParallelIterator<Item = std::io::Result<(&HgPath, Dispatch)>> {
+    dmap.par_iter().map(move |(filename, entry)| {
+        let filename: &HgPath = filename;
+        let meta = match hg_path_to_path_buf(filename) {
+            Ok(p) => root_dir.as_ref().join(p).symlink_metadata(),
+            Err(e) => return Err(e.into()),
+        };
 
-                Some(match meta {
-                    Ok(ref m)
-                        if !(m.file_type().is_file()
-                            || m.file_type().is_symlink()) =>
-                    {
-                        Ok((filename, dispatch_missing(entry.state)))
-                    }
-                    Ok(m) => Ok((filename, dispatch_found(
-                            filename,
-                            *entry,
-                            HgMetadata::from_metadata(m),
-                            &dmap.copy_map,
-                            check_exec,
-                            list_clean,
-                            last_normal_time))),
-                    Err(ref e)
-                        if e.kind() == std::io::ErrorKind::NotFound
-                            || e.raw_os_error() == Some(20) =>
-                    {
-                        // Rust does not yet have an `ErrorKind` for
-                        // `NotADirectory` (errno 20)
-                        // It happens if the dirstate contains `foo/bar` and
-                        // foo is not a directory
-                        Ok((filename, dispatch_missing(entry.state)))
-                    }
-                    Err(e) => Err(e),
-                })
-            },
-        )
-        .collect()
+        match meta {
+            Ok(ref m)
+                if !(m.file_type().is_file()
+                    || m.file_type().is_symlink()) =>
+            {
+                Ok((filename, dispatch_missing(entry.state)))
+            }
+            Ok(m) => Ok((
+                filename,
+                dispatch_found(
+                    filename,
+                    *entry,
+                    HgMetadata::from_metadata(m),
+                    &dmap.copy_map,
+                    check_exec,
+                    list_clean,
+                    last_normal_time,
+                ),
+            )),
+            Err(ref e)
+                if e.kind() == std::io::ErrorKind::NotFound
+                    || e.raw_os_error() == Some(20) =>
+            {
+                // Rust does not yet have an `ErrorKind` for
+                // `NotADirectory` (errno 20)
+                // It happens if the dirstate contains `foo/bar` and
+                // foo is not a directory
+                Ok((filename, dispatch_missing(entry.state)))
+            }
+            Err(e) => Err(e),
+        }
+    })
 }
 
 pub struct StatusResult<'a> {
@@ -210,18 +207,19 @@ 
 
 pub fn status(
     dmap: &DirstateMap,
-    root_dir: impl AsRef<Path> + Sync + Copy,
+    root_dir: impl AsRef<Path> + Sync + Send + Copy,
     list_clean: bool,
     last_normal_time: i64,
     check_exec: bool,
 ) -> std::io::Result<(Vec<&HgPath>, StatusResult)> {
-    let results = stat_dmap_entries(
+    let results: std::io::Result<_> = stat_dmap_entries(
         &dmap,
         root_dir,
         check_exec,
         list_clean,
         last_normal_time,
-    )?;
+    )
+    .collect();
 
-    Ok(build_response(results))
+    Ok(build_response(results?))
 }