Open budde25 opened 9 months ago
~I will dig into this later (hopefully), but in the meantime, what is your use-case for using suspend
? In general, suspend
doesn't play too well with MultiProgress
because it has to pessimistically assume that you wrote to the console in the closure. It can't possibly know how many lines you may have written, so it basically has to "forget" about any progress bars rendered prior to the suspend call.~
^ Ignore this brainfart please
Ok that does make sense. Yes that is basically my use case, however, I was putting debug!
(from log) statements in the closure. So 99% of the time i didn't write anything to the console but was still breaking my progress bars. I suppose i should probably just skip the progress bars when there is verbosity or put the suspend in a if verbosity block.
Thanks!
Ah.... actually, sorry, I took another look and my explanation from before was wrong. suspend
works like this:
ProgressBar
sProgressBar
sThe issue is that your ProgressBar
s are going out of scope at the end of each iteration of the outer for-loop. (Remember that a ProgressBar
is basically just an Arc
).
The solution is to just extend the lifetime of all the ProgressBar
s by keeping a Vec
or something of them in the outer scope:
use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
fn main() {
let m = MultiProgress::new();
let sty = ProgressStyle::with_template(
"{prefix:<10} [{elapsed_precise}] {bar:40.cyan/blue} {pos:>7}/{len:7} {msg}",
)
.unwrap()
.progress_chars("##-");
let mut pbs = vec![];
let total = m.add(ProgressBar::new(10));
total.set_style(sty.clone());
total.set_prefix("total");
for i in 0..10 {
let name = format!("item-{i}");
let pb = m.insert_before(&total, ProgressBar::new(100));
pbs.push(pb.clone());
pb.set_style(sty.clone());
pb.set_prefix(name);
for _ in 0..100 {
total.suspend(|| ());
pb.inc(1);
}
pb.finish();
total.inc(1);
}
total.finish();
}
If you'd like to avoid the manual suspend
calls, you can use something like this with your logging backend btw: https://github.com/Agilent/yb/blob/main/yb/src/main.rs#L72. You still need to keep the ProgressBar
s alive though.
The whole MultiProgress
lifetime thing is something I really need to document better. I will submit an issue against myself to do it. Sorry again for the trouble.
Oh that is awesome, yeah that does seem to fix use case (though it might still be desirable to skip suspend when i can). Thanks for taking a stab at that and finding a solution!
Follow up question. Why doesn't the the Multiprogress bar keep the individual Arc's around by doing a clone into a inner vec itself (thus keeping ref count a 1) and keeping the old bars around?
Oh that is awesome, yeah that does seem to fix use case (though it might still be desirable to skip suspend when i can). Thanks for taking a stab at that and finding a solution!
Sure thing!
Follow up question. Why doesn't the the Multiprogress bar keep the individual Arc's around by doing a clone into a inner vec itself (thus keeping ref count a 1) and keeping the old bars around?
It's a trade-off. We want to be able to redraw as much as possible in the case of a call to suspend
, for instance. But we also don't want the memory usage of MultiProgress
to grow forever in a long-running CLI. The big assumption is that if a ProgressBar
has been dropped, then the user probably doesn't care about drawing it anymore.
Perhaps we could add an option for MultiProgress
to always keep around an Arc
of each ProgressBar
added. This could make for a helpful opt-out of the behavior I just described, for cases when the user doesn't want to have to think about ProgressBar
lifetime and is OK taking the potential hit on memory. And it would make code that like what you originally posted Just Work™️. What do you think @djc?
BTW for some additional context, see:
Do we already have good documentation for the constraints here? I feel like the Drop
-based behavior is pretty natural for Rust, and adding an optional way to artificially keep ProgressBar
s alive feels like a bunch of extra complexity that we could do without.
Do we already have good documentation for the constraints here? I feel like the
Drop
-based behavior is pretty natural for Rust, and adding an optional way to artificially keepProgressBar
s alive feels like a bunch of extra complexity that we could do without.
That's fair. It is not well documented, but I submitted an issue to do so: https://github.com/console-rs/indicatif/issues/595
I noticed that when i added the
suspend
function to my cargo-like multi progress bar it kept deleting previous progress bars.env:
cargo toml:
After narrowing down the issue i found that it only happened if I used the
insert_before
or theinsert_at
function. The regularadd
did not cause this.it gives the output of:
removing the suspend call gives me the expected output of:
When i ran it with add, instead of
insert_before
it printed correctly both ways (expect the total is at the top rather than bottom hence why i wanted to useinsert_before
. I also tried usingpb.suspend()
instead oftotal.suspend()
but got the same results.Thanks!