Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gitoxide progress bar fixes #11800

Merged
merged 2 commits into from
Mar 4, 2023
Merged

gitoxide progress bar fixes #11800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e96095
Adjust git::fetch() progress bar to deal with gitoxide-specifics
Byron Mar 4, 2023
789efe3
Cool down progress loop.
Byron Mar 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ filetime = "0.2.9"
flate2 = { version = "1.0.3", default-features = false, features = ["zlib"] }
git2 = "0.16.0"
git2-curl = "0.17.0"
gix = { version = "0.38.0", default-features = false, features = ["blocking-http-transport-curl", "progress-tree"] }
gix-features-for-configuration-only = { version = "0.27.0", package = "gix-features", features = [ "parallel" ] }
gix = { version = "0.39.0", default-features = false, features = ["blocking-http-transport-curl", "progress-tree"] }
gix-features-for-configuration-only = { version = "0.28.0", package = "gix-features", features = [ "parallel" ] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to ask in the previous PR review. Could you give me a pointer to how this configuration works?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gix-features crate contains all parts of an implementation that are changeable at compile-time with cargo features. Plumbing crates pull it in and if necessary, enable 'base' versions of the capabilities they need. Those are always pure-Rust implementations of algorithms like zlib or SHA1. The gix-features crate also contains primitives for parallelism and utilities to parallelize work.

The gix crate as entry-point for application developers now pulls in gix-features to enable typical features that add on top of the baseline, like enabling actual parallelism with threads, or better performing versions of core algorithms. This works because cargo-feature resolution will compile gix-features only once and aggregate all features set by all crates that use it.

Now, cargo has disabled default features for gix and only re-enables a few ones. None of the features provided by the gix-features crate are 'forwarded' in the gix crate to avoid duplication, so cargo has to pull it in itself to get a chance to set the features it needs. For now, this is only parallel to get parallel computation for everything. but it could also enable faster SHA1 or zlib implementations, but that would pull in more C - right now gitoxide is configured to be 'pure Rust' which should be most portable.

You can try it yourself by commenting out the gix-features-for-configuration-only … line and fetch the crates index - it will be single-threaded then and quite a bit longer. It will still be faster than the git2 version though as it's optimal, and doesn't do any unnecessary computation. It's quite neat to see that the gix_feature::parallel functions also exist in API-compatible serial forms that work the same - those kick in when parallel is not set.

I think @epage called it a 'cargo-features-hack', and that's certainly a fitting name as well 😁.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much! I didn't expect such a comprehensive explanation 😆. Really really useful!

glob = "0.3.0"
hex = "0.4"
hmac = "0.12.1"
Expand Down
41 changes: 29 additions & 12 deletions src/cargo/sources/git/oxide.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,34 @@ fn translate_progress_to_bar(

// We choose `N=10` here to make a `300ms * 10slots ~= 3000ms`
// sliding window for tracking the data transfer rate (in bytes/s).
let mut last_update = Instant::now();
let mut counter = MetricsCounter::<10>::new(0, last_update);
let mut last_percentage_update = Instant::now();
let mut last_fast_update = Instant::now();
let mut counter = MetricsCounter::<10>::new(0, last_percentage_update);

let mut tasks = Vec::with_capacity(10);
let update_interval = std::time::Duration::from_millis(300);
let short_check_interval = Duration::from_millis(50);
let slow_check_interval = std::time::Duration::from_millis(300);
let fast_check_interval = Duration::from_millis(50);
let sleep_interval = Duration::from_millis(10);
debug_assert_eq!(
slow_check_interval.as_millis() % fast_check_interval.as_millis(),
0,
"progress should be smoother by keeping these as multiples of each other"
);
debug_assert_eq!(
fast_check_interval.as_millis() % sleep_interval.as_millis(),
0,
"progress should be smoother by keeping these as multiples of each other"
);

while let Some(root) = root.upgrade() {
let not_yet = last_update.elapsed() < update_interval;
if not_yet {
std::thread::sleep(short_check_interval);
std::thread::sleep(sleep_interval);
let needs_update = last_fast_update.elapsed() >= fast_check_interval;
if !needs_update {
continue;
}
let now = Instant::now();
last_fast_update = now;

root.sorted_snapshot(&mut tasks);

fn progress_by_id(
Expand All @@ -96,13 +112,14 @@ fn translate_progress_to_bar(
tasks.iter().find_map(|(_, t)| cb(t))
}

const NUM_PHASES: usize = 2; // indexing + delta-resolution, both with same amount of objects to handle
if let Some(objs) = find_in(&tasks, |t| progress_by_id(resolve_objects, t)) {
// Resolving deltas.
let objects = objs.step.load(Ordering::Relaxed);
let total_objects = objs.done_at.expect("known amount of objects");
let msg = format!(", ({objects}/{total_objects}) resolving deltas");

progress_bar.tick(objects, total_objects, &msg)?;
progress_bar.tick(total_objects + objects, total_objects * NUM_PHASES, &msg)?;
} else if let Some((objs, read_pack)) =
find_in(&tasks, |t| progress_by_id(read_pack_bytes, t)).and_then(|read| {
find_in(&tasks, |t| progress_by_id(delta_index_objects, t))
Expand All @@ -114,15 +131,15 @@ fn translate_progress_to_bar(
let total_objects = objs.done_at.expect("known amount of objects");
let received_bytes = read_pack.step.load(Ordering::Relaxed);

let now = Instant::now();
if !not_yet {
let needs_percentage_update = last_percentage_update.elapsed() >= slow_check_interval;
if needs_percentage_update {
counter.add(received_bytes, now);
last_update = now;
last_percentage_update = now;
}
let (rate, unit) = human_readable_bytes(counter.rate() as u64);
let msg = format!(", {rate:.2}{unit}/s");

progress_bar.tick(objects, total_objects, &msg)?;
progress_bar.tick(objects, total_objects * NUM_PHASES, &msg)?;
}
}
Ok(())
Expand Down