Skip to content

Commit

Permalink
manifest-tree: fix progress bar for "diff" operation
Browse files Browse the repository at this point in the history
Summary:
Now that the diff is parallelized, the "current depth" progress bar doesn't work well since the depth jumps around depending on ordering between threads. For now switch to a simpler, albeit less useful, approach that shows the number of trees we have processed so far.

I feel like there must be a way to get an approximate average/weighted depth across the entire operation, but I don't want to spend time on this right now.

Reviewed By: zzl0

Differential Revision: D65454850

fbshipit-source-id: d14cd6eff32d71e392f5edbaba1d5f62da131101
  • Loading branch information
muirdm authored and facebook-github-bot committed Nov 5, 2024
1 parent e8ce33b commit 760687c
Showing 1 changed file with 11 additions and 8 deletions.
19 changes: 11 additions & 8 deletions eden/scm/lib/manifest-tree/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use pathmatcher::DirectoryMatch;
use pathmatcher::Matcher;
use progress_model::ActiveProgressBar;
use progress_model::ProgressBar;
use progress_model::Registry;
use types::PathComponentBuf;
use types::RepoPath;
use types::RepoPathBuf;
Expand Down Expand Up @@ -90,6 +91,7 @@ struct DiffWorker {
matcher: Arc<dyn Matcher + Sync + Send>,
store: InnerStore,
pending: Arc<AtomicUsize>,
progress_bar: Arc<ProgressBar>,
}

/// A parallel iterator over two trees.
Expand All @@ -114,13 +116,18 @@ pub fn diff(

let store = left.store.clone();

let progress_bar = ProgressBar::new("diffing manifest", 0, "trees");
let registry = Registry::main();
registry.register_progress_bar(&progress_bar);

let worker = DiffWorker {
work_recv,
work_send,
result_send,
pending: Arc::new(AtomicUsize::new(0)),
store,
matcher: Arc::new(matcher),
progress_bar: progress_bar.clone(),
};

let thread_count = THREAD_POOL.max_count();
Expand All @@ -142,7 +149,7 @@ pub fn diff(

Box::new(DiffIter {
result_recv,
progress_bar: ProgressBar::new_adhoc("diffing tree", 18, "depth"),
progress_bar: ProgressBar::push_active(progress_bar, registry),
})
}

Expand Down Expand Up @@ -183,6 +190,7 @@ impl DiffWorker {
}
acc
});
self.progress_bar.increase_position(keys.len() as u64);
let _ = self.store.prefetch(keys);

// Now process diff work, accumulating work for the next tree level.
Expand Down Expand Up @@ -235,20 +243,15 @@ impl DiffWorker {

struct DiffIter {
result_recv: Receiver<Result<DiffEntry>>,
#[allow(unused)]
progress_bar: ActiveProgressBar,
}

impl Iterator for DiffIter {
type Item = Result<DiffEntry>;

fn next(&mut self) -> Option<Self::Item> {
let next = self.result_recv.recv().ok();
if let Some(Ok(DiffEntry { ref path, .. })) = next {
// Set "depth" according to item depth.
self.progress_bar
.set_position(path.ancestors().count() as u64);
}
next
self.result_recv.recv().ok()
}
}

Expand Down

0 comments on commit 760687c

Please sign in to comment.