From 94822bf22532e4c3c3bf761f9ca04ce905d1d1cc Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 17 Feb 2022 19:09:09 +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 | 23 +++----------------- 4 files changed, 8 insertions(+), 97 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 957cc742..c0fa6839 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, ProgressState, Status}; +use crate::state::{BarState, ProgressState, Status}; use crate::style::ProgressStyle; use crate::{ProgressBarIter, ProgressIterator}; @@ -151,56 +151,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. @@ -355,7 +305,6 @@ impl ProgressBar { self.reset_elapsed(); self.update_and_draw(Instant::now(), |state| { state.pos = 0; - state.last_draw = None; state.status = Status::InProgress; }); } diff --git a/src/state.rs b/src/state.rs index 92ee5058..cd63e518 100644 --- a/src/state.rs +++ b/src/state.rs @@ -128,7 +128,6 @@ impl BarState { } drop(draw_state); - self.state.last_draw = Some((self.state.pos, now)); drawable.draw() } } @@ -153,8 +152,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>, @@ -170,8 +167,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(), @@ -253,19 +248,12 @@ impl ProgressState { let old_pos = self.pos; f(self); let new_pos = self.pos; - if new_pos != old_pos { + let updated = new_pos != old_pos; + if updated { self.est.record_step(new_pos, 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, - } + updated } } @@ -396,11 +384,6 @@ pub(crate) enum Status { DoneHidden, } -pub(crate) enum Limit { - Rate(Duration), - Units(u64), -} - #[cfg(test)] mod tests { use super::*;