From a79c481d087c161145ff07e2d71556a0a1c87145 Mon Sep 17 00:00:00 2001 From: G0rocks Date: Wed, 9 Jul 2025 14:28:22 +0000 Subject: [PATCH 1/5] Add comments for increased understanding. Simplify ProgressState.duration() function to show the duration regardless of if there is no progress or if the progressbar is finished. Rename Estimator.prev_steps to steps_done for increased readability --- src/state.rs | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/state.rs b/src/state.rs index ba472122..41e57c1c 100644 --- a/src/state.rs +++ b/src/state.rs @@ -288,17 +288,20 @@ impl ProgressState { /// The expected ETA pub fn eta(&self) -> Duration { + // If done set ETA to 0 if self.is_finished() { return Duration::new(0, 0); } + // If no bar length set then return 0, otherwise get progress bar length let len = match self.len { Some(len) => len, None => return Duration::new(0, 0), }; + // get current position let pos = self.pos.pos.load(Ordering::Relaxed); - + let sps = self.est.steps_per_second(Instant::now()); // Infinite duration should only ever happen at the beginning, so in this case it's okay to @@ -312,9 +315,6 @@ impl ProgressState { /// The expected total duration (that is, elapsed time + expected ETA) pub fn duration(&self) -> Duration { - if self.len.is_none() || self.is_finished() { - return Duration::new(0, 0); - } self.started.elapsed().saturating_add(self.eta()) } @@ -421,9 +421,9 @@ impl TabExpandedString { pub(crate) struct Estimator { smoothed_steps_per_sec: f64, double_smoothed_steps_per_sec: f64, - prev_steps: u64, - prev_time: Instant, - start_time: Instant, + steps_done: u64, // How many steps have already been accomplished + prev_time: Instant, // The last time a step was made + start_time: Instant, // The instant the process started } impl Estimator { @@ -431,25 +431,25 @@ impl Estimator { Self { smoothed_steps_per_sec: 0.0, double_smoothed_steps_per_sec: 0.0, - prev_steps: 0, + steps_done: 0, prev_time: now, start_time: now, } } - fn record(&mut self, new_steps: u64, now: Instant) { - // sanity check: don't record data if time or steps have not advanced - if new_steps <= self.prev_steps || now <= self.prev_time { - // Reset on backwards seek to prevent breakage from seeking to the end for length determination - // See https://github.com/console-rs/indicatif/issues/480 - if new_steps < self.prev_steps { - self.prev_steps = new_steps; - self.reset(now); - } + // Function that updates the estimator parameters based on the number of new steps and the current time + fn record(&mut self, new_steps_done: u64, now: Instant) { + // sanity check: don't record data if time or steps have not advanced + if new_steps_done <= self.prev_steps || now <= self.prev_time { + // Reset on backwards seek to prevent breakage from seeking to the end for length determination + // See https://github.com/console-rs/indicatif/issues/480 + if new_steps_done < self.steps_done { + self.steps_done = new_steps_done; + self.reset(now); return; } - let delta_steps = new_steps - self.prev_steps; + let delta_steps = new_steps_done - self.steps_done; let delta_t = duration_to_secs(now - self.prev_time); // the rate of steps we saw in this update @@ -474,7 +474,8 @@ impl Estimator { self.double_smoothed_steps_per_sec = self.double_smoothed_steps_per_sec * weight + normalized_smoothed_steps_per_sec * (1.0 - weight); - self.prev_steps = new_steps; + // Update how many steps done and set prev_time to the instant the last progress was made + self.steps_done = new_steps_done; self.prev_time = now; } @@ -484,7 +485,7 @@ impl Estimator { self.smoothed_steps_per_sec = 0.0; self.double_smoothed_steps_per_sec = 0.0; - // only reset prev_time, not prev_steps + // only reset prev_time, not steps_done self.prev_time = now; self.start_time = now; } From 92e50aa504a505efeab5c3418db93a078339b96d Mon Sep 17 00:00:00 2001 From: G0rocks Date: Wed, 9 Jul 2025 14:43:13 +0000 Subject: [PATCH 2/5] Change ProgressState.eta() function to show ETA before any progress has been made in a way where every second could be the second progress gets made so we assume each step needed will take that much time. This shows in an ETA that grows while no progress has been made after starting a progress bar and settles when the first step is made. The "sanity check" in Estimator.record() function needed to be removed for this since it prohibited any updates to Estimator parameters if no progress had been made. The ProgressState.eta() now also includes the time passed since last progress was made in it's ETA calculations and that is what makes the ETA counter count down. Add sec_per_step to Estimator which is an estimate for how many seconds each step takes. Also included sec_per_step in the Estimator.reset() function. Update Estimator.record() function to keep returning the same estimate for how many seconds each step takes even though no progress has been made. This is done so that the ProgressState.eta() function will keep counting down the actual estimated time. An exception is if the time since the last progress is more than the average time per step then we increase the estimate for how many seconds each step takes by 2% everytime Estimator.record() functino is called. This shows an increase in the ETA at a rate that is much more conservative than the previous rate and in the case of no progress being possible (for example if the progress is for a transmission and the router is unplugged) then the user will clearly see the ETA rise with no hope of it going down. In the case that progress does get done (somebody plugged the router back in) then the actual time it took for progress to be made is used to estimate the time for future steps. These changes make the parameters Estimator.smoothed_steps_per_sec and Estimator.double_smoothed_steps_per_sec obsolete and unnecessary as well as some functions potentially, like Estimator.steps_per_second() which uses this double exponential smoothing and seems to be only used in the tests/examples and also in the ProgressBar.per_sec() function which I don't know how much is used so I left everything related to these parameters untouched. --- src/state.rs | 53 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/src/state.rs b/src/state.rs index 41e57c1c..47679ec8 100644 --- a/src/state.rs +++ b/src/state.rs @@ -301,16 +301,12 @@ impl ProgressState { // get current position let pos = self.pos.pos.load(Ordering::Relaxed); + let steps_remaining = len.saturating_sub(pos) as f64; - let sps = self.est.steps_per_second(Instant::now()); - - // Infinite duration should only ever happen at the beginning, so in this case it's okay to - // just show an ETA of 0 until progress starts to occur. - if sps == 0.0 { - return Duration::new(0, 0); - } - - secs_to_duration(len.saturating_sub(pos) as f64 / sps) + // estimate seconds remaining + let secs_remaining = secs_to_duration(self.est.sec_per_step * steps_remaining as f64).saturating_sub(Instant::now() - self.est.prev_time); + // Return estimated time remaining in seconds + return secs_remaining; } /// The expected total duration (that is, elapsed time + expected ETA) @@ -419,6 +415,7 @@ impl TabExpandedString { /// slow asymptotic approach to zero (until the next spike). #[derive(Debug)] pub(crate) struct Estimator { + sec_per_step: f64, // Estimate for how many seconds each step takes on average smoothed_steps_per_sec: f64, double_smoothed_steps_per_sec: f64, steps_done: u64, // How many steps have already been accomplished @@ -429,6 +426,7 @@ pub(crate) struct Estimator { impl Estimator { fn new(now: Instant) -> Self { Self { + sec_per_step: 0.0, smoothed_steps_per_sec: 0.0, double_smoothed_steps_per_sec: 0.0, steps_done: 0, @@ -439,8 +437,6 @@ impl Estimator { // Function that updates the estimator parameters based on the number of new steps and the current time fn record(&mut self, new_steps_done: u64, now: Instant) { - // sanity check: don't record data if time or steps have not advanced - if new_steps_done <= self.prev_steps || now <= self.prev_time { // Reset on backwards seek to prevent breakage from seeking to the end for length determination // See https://github.com/console-rs/indicatif/issues/480 if new_steps_done < self.steps_done { @@ -449,9 +445,43 @@ impl Estimator { return; } + // We use avg_sec_per_step as the average secs per step so far only counting the completed steps before this one + let avg_sec_per_step = self.prev_time.saturating_duration_since(self.start_time).as_secs_f64() / self.steps_done as f64; + let delta_steps = new_steps_done - self.steps_done; let delta_t = duration_to_secs(now - self.prev_time); + // If no progress has been made, assume the possibility of progress in the next second + // Meaning that steps per second is however many seconds have passed since last progress was made + // Note: When the first progress happens the ETA will already be set to what the ETA will be after the progress happens + if delta_steps == 0 { + // If first step in progress, assume progress could happen any second + if self.steps_done == 0 { + self.sec_per_step = (now - self.start_time).as_secs_f64(); + // Set log prev time + self.prev_time = now; + return; + } + // If the time since last progress is more than the average estimated time per step + // Assume the possibility of a full stop having happened and increase the sec_per_step + // To show an everincreasing time the estimated time per step increases by 2% (picked because it seemed to work well) + // every time the function is run + if delta_t > avg_sec_per_step { + self.sec_per_step = self.sec_per_step * 1.02; + return; + } + // Otherwise, assume we're still counting down and change nothing + return; + } + + + // let time_elapsed_before = self.prev_time - self.start_time; + // self.etr = (self.etr * time_elapsed_before.as_secs_f64() + delta_t*delta_t) / (now - self.start_time).as_secs_f64(); + // self.sec_per_step = (avg_sec_per_step * self.steps_done as f64 + (new_steps_done - self.steps_done) as f64 * delta_t/delta_steps as f64) / new_steps_done as f64; + self.sec_per_step = now.saturating_duration_since(self.start_time).as_secs_f64() / new_steps_done as f64; + + // 2025-07-09 G0rocks: Old code not used for ETA estimation anymore but could be used elsewhere. Would like to remove but won't dare as of now. See https://github.com/console-rs/indicatif/pull/721 + //--------------------------------------------------------------------- // the rate of steps we saw in this update let new_steps_per_second = delta_steps as f64 / delta_t; @@ -484,6 +514,7 @@ impl Estimator { pub(crate) fn reset(&mut self, now: Instant) { self.smoothed_steps_per_sec = 0.0; self.double_smoothed_steps_per_sec = 0.0; + self.sec_per_step = 0.0; // only reset prev_time, not steps_done self.prev_time = now; From 46f0b4f84966eb4ce68b8b1263ec7d6af176a287 Mon Sep 17 00:00:00 2001 From: G0rocks Date: Wed, 16 Jul 2025 11:43:18 +0000 Subject: [PATCH 3/5] Implement suggestions from cargo fmt --all -- --check --- src/state.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/state.rs b/src/state.rs index 47679ec8..9d059a0f 100644 --- a/src/state.rs +++ b/src/state.rs @@ -304,7 +304,8 @@ impl ProgressState { let steps_remaining = len.saturating_sub(pos) as f64; // estimate seconds remaining - let secs_remaining = secs_to_duration(self.est.sec_per_step * steps_remaining as f64).saturating_sub(Instant::now() - self.est.prev_time); + let secs_remaining = secs_to_duration(self.est.sec_per_step * steps_remaining as f64) + .saturating_sub(Instant::now() - self.est.prev_time); // Return estimated time remaining in seconds return secs_remaining; } @@ -415,12 +416,12 @@ impl TabExpandedString { /// slow asymptotic approach to zero (until the next spike). #[derive(Debug)] pub(crate) struct Estimator { - sec_per_step: f64, // Estimate for how many seconds each step takes on average + sec_per_step: f64, // Estimate for how many seconds each step takes on average smoothed_steps_per_sec: f64, double_smoothed_steps_per_sec: f64, - steps_done: u64, // How many steps have already been accomplished - prev_time: Instant, // The last time a step was made - start_time: Instant, // The instant the process started + steps_done: u64, // How many steps have already been accomplished + prev_time: Instant, // The last time a step was made + start_time: Instant, // The instant the process started } impl Estimator { @@ -446,7 +447,11 @@ impl Estimator { } // We use avg_sec_per_step as the average secs per step so far only counting the completed steps before this one - let avg_sec_per_step = self.prev_time.saturating_duration_since(self.start_time).as_secs_f64() / self.steps_done as f64; + let avg_sec_per_step = self + .prev_time + .saturating_duration_since(self.start_time) + .as_secs_f64() + / self.steps_done as f64; let delta_steps = new_steps_done - self.steps_done; let delta_t = duration_to_secs(now - self.prev_time); @@ -474,11 +479,11 @@ impl Estimator { return; } - // let time_elapsed_before = self.prev_time - self.start_time; // self.etr = (self.etr * time_elapsed_before.as_secs_f64() + delta_t*delta_t) / (now - self.start_time).as_secs_f64(); // self.sec_per_step = (avg_sec_per_step * self.steps_done as f64 + (new_steps_done - self.steps_done) as f64 * delta_t/delta_steps as f64) / new_steps_done as f64; - self.sec_per_step = now.saturating_duration_since(self.start_time).as_secs_f64() / new_steps_done as f64; + self.sec_per_step = + now.saturating_duration_since(self.start_time).as_secs_f64() / new_steps_done as f64; // 2025-07-09 G0rocks: Old code not used for ETA estimation anymore but could be used elsewhere. Would like to remove but won't dare as of now. See https://github.com/console-rs/indicatif/pull/721 //--------------------------------------------------------------------- From 60af1d1b94329b0d38397bd560920c62c0ecada2 Mon Sep 17 00:00:00 2001 From: G0rocks Date: Wed, 16 Jul 2025 15:38:35 +0000 Subject: [PATCH 4/5] Implement suggestions by cargo fmt --all -- --check --- src/state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/state.rs b/src/state.rs index 9d059a0f..6c61d42d 100644 --- a/src/state.rs +++ b/src/state.rs @@ -302,10 +302,10 @@ impl ProgressState { // get current position let pos = self.pos.pos.load(Ordering::Relaxed); let steps_remaining = len.saturating_sub(pos) as f64; - + // estimate seconds remaining let secs_remaining = secs_to_duration(self.est.sec_per_step * steps_remaining as f64) - .saturating_sub(Instant::now() - self.est.prev_time); + .saturating_sub(Instant::now() - self.est.prev_time); // Return estimated time remaining in seconds return secs_remaining; } From 18828be7402c249fb638d8a87677fc2563e92fbc Mon Sep 17 00:00:00 2001 From: G0rocks Date: Wed, 16 Jul 2025 16:52:26 +0000 Subject: [PATCH 5/5] Implement suggestions by cargo fmt --all -- --check round 3 --- src/state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/state.rs b/src/state.rs index 6c61d42d..e50909e9 100644 --- a/src/state.rs +++ b/src/state.rs @@ -304,10 +304,10 @@ impl ProgressState { let steps_remaining = len.saturating_sub(pos) as f64; // estimate seconds remaining - let secs_remaining = secs_to_duration(self.est.sec_per_step * steps_remaining as f64) + let secs_remaining = secs_to_duration(self.est.sec_per_step * steps_remaining) .saturating_sub(Instant::now() - self.est.prev_time); // Return estimated time remaining in seconds - return secs_remaining; + secs_remaining } /// The expected total duration (that is, elapsed time + expected ETA) @@ -472,7 +472,7 @@ impl Estimator { // To show an everincreasing time the estimated time per step increases by 2% (picked because it seemed to work well) // every time the function is run if delta_t > avg_sec_per_step { - self.sec_per_step = self.sec_per_step * 1.02; + self.sec_per_step *= 1.02; return; } // Otherwise, assume we're still counting down and change nothing