From ca2819626d5afdbc70e02e9e72d8fca4e8803bb4 Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Wed, 13 Feb 2019 16:53:05 -0800 Subject: [PATCH] Review feedback. --- src/rust/engine/fs/src/snapshot.rs | 6 ++++-- src/rust/engine/fs/src/store.rs | 14 ++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/rust/engine/fs/src/snapshot.rs b/src/rust/engine/fs/src/snapshot.rs index c91328d935a4..eaa1eca90da4 100644 --- a/src/rust/engine/fs/src/snapshot.rs +++ b/src/rust/engine/fs/src/snapshot.rs @@ -86,8 +86,10 @@ impl Snapshot { future::ok(path_stats).to_boxed() }) .map(move |path_stats_per_directory| { - let path_stats = - Iterator::flatten(path_stats_per_directory.into_iter().map(|v| v.into_iter())).collect(); + let mut path_stats = + Iterator::flatten(path_stats_per_directory.into_iter().map(|v| v.into_iter())) + .collect::>(); + path_stats.sort_by(|l, r| l.path().cmp(&r.path())); Snapshot { digest, path_stats } }) .to_boxed() diff --git a/src/rust/engine/fs/src/store.rs b/src/rust/engine/fs/src/store.rs index d6015d910614..412be0870441 100644 --- a/src/rust/engine/fs/src/store.rs +++ b/src/rust/engine/fs/src/store.rs @@ -589,6 +589,9 @@ impl Store { /// Given the Digest for a Directory, recursively walk the Directory, calling the given function /// with the path so far, and the new Directory. /// + /// The recursive walk will proceed concurrently, so if order matters, a caller should sort the + /// output after the call. + /// pub fn walk< T: Send + 'static, F: Fn( @@ -598,13 +601,14 @@ impl Store { &bazel_protos::remote_execution::Directory, ) -> BoxFuture + Send + + Sync + 'static, >( &self, digest: Digest, f: F, ) -> BoxFuture, String> { - let f = Arc::new(Mutex::new(f)); + let f = Arc::new(f); let accumulator = Arc::new(Mutex::new(Vec::new())); self .walk_helper(digest, PathBuf::new(), f, accumulator.clone()) @@ -625,12 +629,13 @@ impl Store { &bazel_protos::remote_execution::Directory, ) -> BoxFuture + Send + + Sync + 'static, >( &self, digest: Digest, path_so_far: PathBuf, - f: Arc>, + f: Arc, accumulator: Arc>>, ) -> BoxFuture<(), String> { let store = self.clone(); @@ -638,10 +643,7 @@ impl Store { .load_directory(digest) .and_then(move |maybe_directory| match maybe_directory { Some(directory) => { - let result_for_directory = { - let f = f.lock(); - f(&store, &path_so_far, digest, &directory) - }; + let result_for_directory = f(&store, &path_so_far, digest, &directory); result_for_directory .and_then(move |r| { {