D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Closed by commit rHGf8a9922a02cb: rust-status: move to recursive traversal to 
prepare for parallel traversal (authored by Alphare).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs 
Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8215?vs=20561=20729

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

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

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

CHANGE DETAILS

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
@@ -26,7 +26,6 @@
 };
 use lazy_static::lazy_static;
 use rayon::prelude::*;
-use std::collections::VecDeque;
 use std::{
 borrow::Cow,
 collections::HashSet,
@@ -38,7 +37,7 @@
 
 /// Wrong type of file from a `BadMatch`
 /// Note: a lot of those don't exist on all platforms.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub enum BadType {
 CharacterDevice,
 BlockDevice,
@@ -63,7 +62,7 @@
 }
 
 /// Was explicitly matched but cannot be found/accessed
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub enum BadMatch {
 OsError(i32),
 BadType(BadType),
@@ -72,7 +71,7 @@
 /// Marker enum used to dispatch new status entries into the right collections.
 /// Is similar to `crate::EntryState`, but represents the transient state of
 /// entries during the lifetime of a command.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 enum Dispatch {
 Unsure,
 Modified,
@@ -300,49 +299,53 @@
 pub list_ignored: bool,
 }
 
-/// Dispatch a single file found during `traverse`.
-/// If `file` is a folder that needs to be traversed, it will be pushed into
+/// Dispatch a single entry (file, folder, symlink...) found during `traverse`.
+/// If the entry is a folder that needs to be traversed, it will be pushed into
 /// `work`.
-fn traverse_worker<'a>(
-work:  VecDeque,
-matcher:  Matcher,
+fn handle_traversed_entry<'a>(
+dir_entry: ,
+matcher: &(impl Matcher + Sync),
+root_dir: impl AsRef,
 dmap: ,
 filename: impl AsRef,
-dir_entry: ,
-ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
-dir_ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
+old_results: , Dispatch>,
+ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
+dir_ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
 options: StatusOptions,
-) -> Option, Dispatch)>> {
-let file_type = match dir_entry.file_type() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
+) -> IoResult, Dispatch)>> {
+let file_type = dir_entry.file_type()?;
 let filename = filename.as_ref();
 let entry_option = dmap.get(filename);
 
 if file_type.is_dir() {
 // Do we need to traverse it?
 if !ignore_fn() || options.list_ignored {
-work.push_front(filename.to_owned());
+return traverse_dir(
+matcher,
+root_dir,
+dmap,
+filename.to_owned(),
+_results,
+ignore_fn,
+dir_ignore_fn,
+options,
+);
 }
 // Nested `if` until `rust-lang/rust#53668` is stable
 if let Some(entry) = entry_option {
 // Used to be a file, is now a folder
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
 } else if file_type.is_file() || file_type.is_symlink() {
 if let Some(entry) = entry_option {
 if matcher.matches_everything() || matcher.matches() {
-let metadata = match dir_entry.metadata() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
-return Some(Ok((
+let metadata = dir_entry.metadata()?;
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_found(
 ,
@@ -351,7 +354,7 @@
 _map,
 options,
 ),
-)));
+)]);
 }
 } else if (matcher.matches_everything() || matcher.matches())
 && !ignore_fn()
@@ -360,113 +363,105 @@
 && dir_ignore_fn()
 {
 if options.list_ignored {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Ignored,
-)));
+   

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare updated this revision to Diff 20561.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8215?vs=20500=20561

BRANCH
  default

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

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

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

CHANGE DETAILS

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
@@ -26,7 +26,6 @@
 };
 use lazy_static::lazy_static;
 use rayon::prelude::*;
-use std::collections::VecDeque;
 use std::{
 borrow::Cow,
 collections::HashSet,
@@ -38,7 +37,7 @@
 
 /// Wrong type of file from a `BadMatch`
 /// Note: a lot of those don't exist on all platforms.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub enum BadType {
 CharacterDevice,
 BlockDevice,
@@ -63,7 +62,7 @@
 }
 
 /// Was explicitly matched but cannot be found/accessed
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub enum BadMatch {
 OsError(i32),
 BadType(BadType),
@@ -72,7 +71,7 @@
 /// Marker enum used to dispatch new status entries into the right collections.
 /// Is similar to `crate::EntryState`, but represents the transient state of
 /// entries during the lifetime of a command.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 enum Dispatch {
 Unsure,
 Modified,
@@ -300,49 +299,53 @@
 pub list_ignored: bool,
 }
 
-/// Dispatch a single file found during `traverse`.
-/// If `file` is a folder that needs to be traversed, it will be pushed into
+/// Dispatch a single entry (file, folder, symlink...) found during `traverse`.
+/// If the entry is a folder that needs to be traversed, it will be pushed into
 /// `work`.
-fn traverse_worker<'a>(
-work:  VecDeque,
-matcher:  Matcher,
+fn handle_traversed_entry<'a>(
+dir_entry: ,
+matcher: &(impl Matcher + Sync),
+root_dir: impl AsRef,
 dmap: ,
 filename: impl AsRef,
-dir_entry: ,
-ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
-dir_ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
+old_results: , Dispatch>,
+ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
+dir_ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
 options: StatusOptions,
-) -> Option, Dispatch)>> {
-let file_type = match dir_entry.file_type() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
+) -> IoResult, Dispatch)>> {
+let file_type = dir_entry.file_type()?;
 let filename = filename.as_ref();
 let entry_option = dmap.get(filename);
 
 if file_type.is_dir() {
 // Do we need to traverse it?
 if !ignore_fn() || options.list_ignored {
-work.push_front(filename.to_owned());
+return traverse_dir(
+matcher,
+root_dir,
+dmap,
+filename.to_owned(),
+_results,
+ignore_fn,
+dir_ignore_fn,
+options,
+);
 }
 // Nested `if` until `rust-lang/rust#53668` is stable
 if let Some(entry) = entry_option {
 // Used to be a file, is now a folder
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
 } else if file_type.is_file() || file_type.is_symlink() {
 if let Some(entry) = entry_option {
 if matcher.matches_everything() || matcher.matches() {
-let metadata = match dir_entry.metadata() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
-return Some(Ok((
+let metadata = dir_entry.metadata()?;
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_found(
 ,
@@ -351,7 +354,7 @@
 _map,
 options,
 ),
-)));
+)]);
 }
 } else if (matcher.matches_everything() || matcher.matches())
 && !ignore_fn()
@@ -360,113 +363,105 @@
 && dir_ignore_fn()
 {
 if options.list_ignored {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Ignored,
-)));
+)]);
 }
 } else {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Unknown,
- 

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare added a comment.


  In D8215#122642 , @Alphare wrote:
  
  > I still don't have a clear solution for the modulepolicy thing, because 
everything is broken if we use strict policies.
  
  It appears that I got confused by an unrelated issue. @marmoute pushed a fix 
for this as D8245 .

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: marmoute, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare added a comment.


  In D8215#122325 , @durin42 wrote:
  
  > 
  
  And it looks like `test-dirstate-race.t` is just broken by this series?
  
  I can't reproduce that however. Do you have a particular way of doing so? 
Because it's a race test... it might be flaky.
  
  I still don't have a clear solution for the modulepolicy thing, because 
everything is broken if we use strict policies.

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: marmoute, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare added a comment.
Alphare added a subscriber: marmoute.


  In D8215#122325 , @durin42 wrote:
  
  > 1. I get test-check-rust-format.t errors
  
  Indeed. It appears that none of the machines I use to run the entire test 
suite (so... not my laptop) have rustfmt installed, which skips this test. I 
have updated the series to fix this issue, and will install rustfmt where 
needed.
  
  > 2. I don't seem to be having any luck running the tests. It looks like the 
rust module policy doesn't cause the right things to get installed? In 
particular, `HGWITHRUSTEXT=cpython HGMODULEPOLICY=rust+c make clean tests` 
flunks `test-remotefilelog-datapack.py` because it can't import parsers.so? And 
it looks like `test-dirstate-race.t` is just broken by this series?
  
  I will look into this, this is bad.
  
  > 3. The re2-missing fallback message breaks tests for users that want rust 
things, but lack re2. This isn't going to work, I'm afraid.
  
  @marmoute suggested I do something in `debuginstall` to check for `re2` on 
the Rust side instead, I think it's a good idea.
  
  > The series is so big it's not practical for me to figure out which 
revision(s) I should be commenting on, so hopefully that's actionable 
feedback...
  
  It is, thanks!

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: marmoute, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare updated this revision to Diff 20500.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8215?vs=20465=20500

BRANCH
  default

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

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

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

CHANGE DETAILS

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
@@ -26,7 +26,6 @@
 };
 use lazy_static::lazy_static;
 use rayon::prelude::*;
-use std::collections::VecDeque;
 use std::{
 borrow::Cow,
 collections::HashSet,
@@ -300,49 +299,53 @@
 pub list_ignored: bool,
 }
 
-/// Dispatch a single file found during `traverse`.
-/// If `file` is a folder that needs to be traversed, it will be pushed into
+/// Dispatch a single entry (file, folder, symlink...) found during `traverse`.
+/// If the entry is a folder that needs to be traversed, it will be pushed into
 /// `work`.
-fn traverse_worker<'a>(
-work:  VecDeque,
-matcher:  Matcher,
+fn handle_traversed_entry<'a>(
+dir_entry: ,
+matcher: &(impl Matcher + Sync),
+root_dir: impl AsRef,
 dmap: ,
 filename: impl AsRef,
-dir_entry: ,
-ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
-dir_ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
+old_results: , Dispatch>,
+ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
+dir_ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
 options: StatusOptions,
-) -> Option, Dispatch)>> {
-let file_type = match dir_entry.file_type() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
+) -> IoResult, Dispatch)>> {
+let file_type = dir_entry.file_type()?;
 let filename = filename.as_ref();
 let entry_option = dmap.get(filename);
 
 if file_type.is_dir() {
 // Do we need to traverse it?
 if !ignore_fn() || options.list_ignored {
-work.push_front(filename.to_owned());
+return traverse_dir(
+matcher,
+root_dir,
+dmap,
+filename.to_owned(),
+_results,
+ignore_fn,
+dir_ignore_fn,
+options,
+);
 }
 // Nested `if` until `rust-lang/rust#53668` is stable
 if let Some(entry) = entry_option {
 // Used to be a file, is now a folder
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
 } else if file_type.is_file() || file_type.is_symlink() {
 if let Some(entry) = entry_option {
 if matcher.matches_everything() || matcher.matches() {
-let metadata = match dir_entry.metadata() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
-return Some(Ok((
+let metadata = dir_entry.metadata()?;
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_found(
 ,
@@ -351,7 +354,7 @@
 _map,
 options,
 ),
-)));
+)]);
 }
 } else if (matcher.matches_everything() || matcher.matches())
 && !ignore_fn()
@@ -360,28 +363,28 @@
 && dir_ignore_fn()
 {
 if options.list_ignored {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Ignored,
-)));
+)]);
 }
 } else {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Unknown,
-)));
+)]);
 }
 }
 } else if let Some(entry) = entry_option {
 // Used to be a file or a folder, now something else.
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
-None
+return Ok(vec![]);
 }
 
 /// Walk the working directory recursively to look for changes compared to the
@@ -397,79 +400,99 @@
 options: StatusOptions,
 ) -> IoResult, Dispatch>> {
 let root_dir = root_dir.as_ref();
-let mut new_results = 

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[durin42 (Augie Fackler)]

durin42 added a comment.


  I'm broadly happy with this series, but:
  
  1. I get test-check-rust-format.t errors
  2. I don't seem to be having any luck running the tests. It looks like the 
rust module policy doesn't cause the right things to get installed? In 
particular, `HGWITHRUSTEXT=cpython HGMODULEPOLICY=rust+c make clean tests` 
flunks `test-remotefilelog-datapack.py` because it can't import parsers.so? And 
it looks like `test-dirstate-race.t` is just broken by this series?
  3. The re2-missing fallback message breaks tests for users that want rust 
things, but lack re2. This isn't going to work, I'm afraid.
  
  The series is so big it's not practical for me to figure out which 
revision(s) I should be commenting on, so hopefully that's actionable 
feedback...

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare updated this revision to Diff 20465.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8215?vs=20458=20465

BRANCH
  default

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

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

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

CHANGE DETAILS

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
@@ -26,7 +26,6 @@
 };
 use lazy_static::lazy_static;
 use rayon::prelude::*;
-use std::collections::VecDeque;
 use std::{
 borrow::Cow,
 collections::HashSet,
@@ -299,49 +298,53 @@
 pub list_ignored: bool,
 }
 
-/// Dispatch a single file found during `traverse`.
-/// If `file` is a folder that needs to be traversed, it will be pushed into
+/// Dispatch a single entry (file, folder, symlink...) found during `traverse`.
+/// If the entry is a folder that needs to be traversed, it will be pushed into
 /// `work`.
-fn traverse_worker<'a>(
-work:  VecDeque,
-matcher:  Matcher,
+fn handle_traversed_entry<'a>(
+dir_entry: ,
+matcher: &(impl Matcher + Sync),
+root_dir: impl AsRef,
 dmap: ,
 filename: impl AsRef,
-dir_entry: ,
-ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
-dir_ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
+old_results: , Dispatch>,
+ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
+dir_ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
 options: StatusOptions,
-) -> Option, Dispatch)>> {
-let file_type = match dir_entry.file_type() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
+) -> IoResult, Dispatch)>> {
+let file_type = dir_entry.file_type()?;
 let filename = filename.as_ref();
 let entry_option = dmap.get(filename);
 
 if file_type.is_dir() {
 // Do we need to traverse it?
 if !ignore_fn() || options.list_ignored {
-work.push_front(filename.to_owned());
+return traverse_dir(
+matcher,
+root_dir,
+dmap,
+filename.to_owned(),
+_results,
+ignore_fn,
+dir_ignore_fn,
+options,
+);
 }
 // Nested `if` until `rust-lang/rust#53668` is stable
 if let Some(entry) = entry_option {
 // Used to be a file, is now a folder
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
 } else if file_type.is_file() || file_type.is_symlink() {
 if let Some(entry) = entry_option {
 if matcher.matches_everything() || matcher.matches() {
-let metadata = match dir_entry.metadata() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
-return Some(Ok((
+let metadata = dir_entry.metadata()?;
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_found(
 ,
@@ -350,7 +353,7 @@
 _map,
 options,
 ),
-)));
+)]);
 }
 } else if (matcher.matches_everything() || matcher.matches())
 && !ignore_fn()
@@ -359,28 +362,28 @@
 && dir_ignore_fn()
 {
 if options.list_ignored {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Ignored,
-)));
+)]);
 }
 } else {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Unknown,
-)));
+)]);
 }
 }
 } else if let Some(entry) = entry_option {
 // Used to be a file or a folder, now something else.
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
-None
+return Ok(vec![]);
 }
 
 /// Walk the working directory recursively to look for changes compared to the
@@ -396,79 +399,99 @@
 options: StatusOptions,
 ) -> IoResult, Dispatch>> {
 let root_dir = root_dir.as_ref();
-let mut new_results = 

D8215: rust-status: move to recursive traversal to prepare for parallel traversal

[Raphaël Gomès]

Alphare created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I have looked into traversing the working directory in parallel either
  by a recursive or an iterative algorithm. The recursive approach won quite
  decisively both in terms of performance and code readability.
  
  You can look at my experiment here:
  https://heptapod.octobus.net/Alphare/rayon-recursive-traversal
  
  The chance of a stack overflow happening because the directories get too 
nested
  seems slim.
  
  This change does not yet do anything in parallel.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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

CHANGE DETAILS

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
@@ -26,7 +26,6 @@
 };
 use lazy_static::lazy_static;
 use rayon::prelude::*;
-use std::collections::VecDeque;
 use std::{
 borrow::Cow,
 collections::HashSet,
@@ -299,48 +298,51 @@
 pub list_ignored: bool,
 }
 
-/// Dispatch a single file found during `traverse`.
-/// If `file` is a folder that needs to be traversed, it will be pushed into
+/// Dispatch a single entry (file, folder, symlink...) found during `traverse`.
+/// If the entry is a folder that needs to be traversed, it will be pushed into
 /// `work`.
-fn traverse_worker<'a>(
-work:  VecDeque,
-matcher:  Matcher,
+fn handle_traversed_entry<'a>(
+dir_entry: ,
+matcher: &(impl Matcher + Sync),
+root_dir: impl AsRef,
 dmap: ,
 filename: impl AsRef,
-dir_entry: ,
-ignore_fn:  for<'r> Fn(&'r HgPath) -> bool,
+old_results: , Dispatch>,
+ignore_fn: &(impl for<'r> Fn(&'r HgPath) -> bool + Sync),
 options: StatusOptions,
-) -> Option, Dispatch)>> {
-let file_type = match dir_entry.file_type() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
+) -> IoResult, Dispatch)>> {
+let file_type = dir_entry.file_type()?;
 let filename = filename.as_ref();
 let entry_option = dmap.get(filename);
 
 if file_type.is_dir() {
 // Do we need to traverse it?
 if !ignore_fn() || options.list_ignored {
-work.push_front(filename.to_owned());
+return traverse_dir(
+matcher,
+root_dir,
+dmap,
+filename.to_owned(),
+_results,
+ignore_fn,
+options,
+);
 }
 // Nested `if` until `rust-lang/rust#53668` is stable
 if let Some(entry) = entry_option {
 // Used to be a file, is now a folder
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
 } else if file_type.is_file() || file_type.is_symlink() {
 if let Some(entry) = entry_option {
 if matcher.matches_everything() || matcher.matches() {
-let metadata = match dir_entry.metadata() {
-Ok(x) => x,
-Err(e) => return Some(Err(e.into())),
-};
-return Some(Ok((
+let metadata = dir_entry.metadata()?;
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_found(
 ,
@@ -349,31 +351,31 @@
 _map,
 options,
 ),
-)));
+)]);
 }
 } else if (matcher.matches_everything() || matcher.matches())
 && !ignore_fn()
 {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Unknown,
-)));
+)]);
 } else if ignore_fn() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 Dispatch::Ignored,
-)));
+)]);
 }
 } else if let Some(entry) = entry_option {
 // Used to be a file or a folder, now something else.
 if matcher.matches_everything() || matcher.matches() {
-return Some(Ok((
+return Ok(vec![(
 Cow::Owned(filename.to_owned()),
 dispatch_missing(entry.state),
-)));
+)]);
 }
 }
-None
+return Ok(vec![]);
 }
 
 /// Walk the working directory recursively to look for changes compared to the
@@ -388,73 +390,96 @@
 options: StatusOptions,
 ) -> IoResult, Dispatch>> {
 let root_dir =