From 25baf1f89993f0ab851930920b0060aca92426de Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 18 Feb 2022 09:49:56 +0100 Subject: [PATCH] Remove draw limiting from the ProgressBar state We now have rate limiting in the draw target, which is takes care of most of the costs of drawing. Limiting earlier is not as helpful in terms of performance, and reduces accuracy. --- examples/fastbar.rs | 11 ++++------ src/multi.rs | 18 --------------- src/progress_bar.rs | 53 +-------------------------------------------- src/state.rs | 28 +++++------------------- 4 files changed, 10 insertions(+), 100 deletions(-) diff --git a/examples/fastbar.rs b/examples/fastbar.rs index c6ea7545..dd15a3e6 100644 --- a/examples/fastbar.rs +++ b/examples/fastbar.rs @@ -2,11 +2,8 @@ use std::time::Instant; use indicatif::{HumanDuration, ProgressBar}; -fn many_units_of_easy_work(n: u64, label: &str, draw_delta: Option) { +fn many_units_of_easy_work(n: u64, label: &str) { let pb = ProgressBar::new(n); - if let Some(v) = draw_delta { - pb.set_draw_delta(v); - } let mut sum = 0; let started = Instant::now(); @@ -31,13 +28,13 @@ fn main() { // Perform a long sequence of many simple computations monitored by a // default progress bar. - many_units_of_easy_work(N, "Default progress bar ", None); + many_units_of_easy_work(N, "Default progress bar "); // Perform the same sequence of many simple computations, but only redraw // after each 0.005% of additional progress. - many_units_of_easy_work(N, "Draw delta is 0.005% ", Some(N / (5 * 100000))); + many_units_of_easy_work(N, "Draw delta is 0.005% "); // Perform the same sequence of many simple computations, but only redraw // after each 0.01% of additional progress. - many_units_of_easy_work(N, "Draw delta is 0.01% ", Some(N / 10000)); + many_units_of_easy_work(N, "Draw delta is 0.01% "); } diff --git a/src/multi.rs b/src/multi.rs index 35899ec9..584c3eba 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -327,24 +327,6 @@ enum InsertLocation<'a> { mod tests { use crate::{MultiProgress, ProgressBar, ProgressDrawTarget}; - #[test] - fn test_draw_delta_deadlock() { - // see issue #187 - let mpb = MultiProgress::new(); - let pb = mpb.add(ProgressBar::new(1)); - pb.set_draw_delta(2); - drop(pb); - } - - #[test] - fn test_abandon_deadlock() { - let mpb = MultiProgress::new(); - let pb = mpb.add(ProgressBar::new(1)); - pb.set_draw_delta(2); - pb.abandon(); - drop(pb); - } - #[test] fn late_pb_drop() { let pb = ProgressBar::new(10); diff --git a/src/progress_bar.rs b/src/progress_bar.rs index 769b80ef..ab54db71 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -7,7 +7,7 @@ use std::thread; use std::time::{Duration, Instant}; use crate::draw_target::ProgressDrawTarget; -use crate::state::{BarState, Limit, ProgressFinish, ProgressState, Status}; +use crate::state::{BarState, ProgressFinish, ProgressState, Status}; use crate::style::ProgressStyle; use crate::{ProgressBarIter, ProgressIterator}; @@ -169,56 +169,6 @@ impl ProgressBar { self.enable_steady_tick(0); } - /// Limit redrawing of progress bar to every `n` steps - /// - /// By default, the progress bar will redraw whenever its state advances. This setting is - /// helpful in situations where the overhead of redrawing the progress bar dominates the - /// computation whose progress is being reported. Setting the draw delta will override - /// the draw rate, setting it to `0`. - /// - /// If `n` is greater than 0, operations that change the progress bar such as - /// [`ProgressBar::tick()`], [`ProgressBar::set_message()`] and [`ProgressBar::set_length()`] - /// will no longer cause the progress bar to be redrawn, and will only be shown once the - /// position advances by `n` steps. - /// - /// ```rust,no_run - /// # use indicatif::ProgressBar; - /// let n = 1_000_000; - /// let pb = ProgressBar::new(n); - /// pb.set_draw_delta(n / 100); // redraw every 1% of additional progress - /// ``` - /// - /// Note that `ProgressDrawTarget` may impose additional buffering of redraws. - pub fn set_draw_delta(&self, gap: u64) { - let mut state = self.state.lock().unwrap(); - state.state.draw_limit = Limit::Units(gap); - } - - /// Sets the refresh rate of progress bar to `n` updates per seconds - /// - /// By default, the progress bar will redraw at most 100 times per second. To disable - /// this behavior, set the draw rate to 0. The draw rate will be ignored if the draw - /// delta is set. - /// - /// This setting automatically adapts the progress bar to a constant refresh rate - /// regardless of how consistent the progress is. - /// - /// This parameter takes precedence on `set_draw_delta` if different from 0. - /// - /// ```rust,no_run - /// # use indicatif::ProgressBar; - /// let n = 1_000_000; - /// let pb = ProgressBar::new(n); - /// pb.set_draw_rate(25); // aims at redrawing at most 25 times per seconds. - /// ``` - /// - /// Note that the [`ProgressDrawTarget`] may impose additional buffering of redraws. - pub fn set_draw_rate(&self, n: u64) { - let mut state = self.state.lock().unwrap(); - let interval = Duration::from_nanos(1_000_000_000 / n); - state.state.draw_limit = Limit::Rate(interval); - } - /// Manually ticks the spinner or progress bar /// /// This automatically happens on any other change to a progress bar. @@ -369,7 +319,6 @@ impl ProgressBar { self.reset_elapsed(); self.state().update(Instant::now(), false, |state| { state.pos = 0; - state.last_draw = None; state.status = Status::InProgress; }); } diff --git a/src/state.rs b/src/state.rs index c21667f4..89631844 100644 --- a/src/state.rs +++ b/src/state.rs @@ -102,7 +102,6 @@ impl BarState { } drop(draw_state); - self.state.last_draw = Some((self.state.pos, now)); drawable.draw() } } @@ -126,8 +125,6 @@ pub struct ProgressState { pub(crate) started: Instant, pub(crate) message: Cow<'static, str>, pub(crate) prefix: Cow<'static, str>, - pub(crate) draw_limit: Limit, - pub(crate) last_draw: Option<(u64, Instant)>, pub(crate) status: Status, pub(crate) est: Estimate, pub(crate) tick_thread: Option>, @@ -142,8 +139,6 @@ impl ProgressState { pos: 0, len, tick: 0, - draw_limit: Limit::Rate(Duration::from_millis(10)), - last_draw: None, status: Status::InProgress, started: Instant::now(), est: Estimate::new(), @@ -220,22 +215,14 @@ impl ProgressState { /// Call the provided `FnOnce` to update the state. If a draw should be run, returns `true`. pub(crate) fn update(&mut self, now: Instant, f: F) -> bool { - let old_pos = self.pos; + let old = (self.pos, self.tick); f(self); - let new_pos = self.pos; - if new_pos != old_pos { - self.est.record_step(new_pos, now); + let new = (self.pos, self.tick); + if new.0 != old.0 { + self.est.record_step(new.0, now); } - let (last_pos, last_time) = match self.last_draw { - Some((pos, last_draw)) => (pos, last_draw), - None => return true, - }; - - match self.draw_limit { - Limit::Rate(interval) => (now - last_time) >= interval, - Limit::Units(gap) => (new_pos - last_pos) >= gap, - } + old != new } } @@ -408,11 +395,6 @@ pub(crate) enum Status { DoneHidden, } -pub(crate) enum Limit { - Rate(Duration), - Units(u64), -} - #[cfg(test)] mod tests { use super::*;