console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.21k stars 238 forks source link

fix members of a multibar always rendering #630

Open remi-dupre opened 4 months ago

remi-dupre commented 4 months ago

Hi!

I noticed a rather inconvenient performance issue with MultiBar, contrary to regular progress bar no rate limiter will prevent computing the string that will displayed for a given member ProgressBar. The cost of this "hidden rendering" can quickly add up if unicode+ANSI decoding features are enabled.

My implementation relies on actually rejecting calls to DrawTarget::drawable for multi-bar targets but marking the member as "delayed" for later redraw. When a call for drawable is actually accepted, all delayed member or first redrawn.

Here is an example flamegraph from the application I'm building, using the main branch :

flamegraph-main

A recurring call to set_length takes 30% of the running time in the example above. With my patch it is down to <4% :

flamegraph-fork

djc commented 4 months ago

This seems like a pretty roundabout way of doing it. Could we instead keep the Arc<Mutex<BarState>> in MultiStateMember and let all the members draw only when the MultiState::draw_target rate limit allows it?

remi-dupre commented 4 months ago

Think I get what you mean, I'll try something :+1:

remi-dupre commented 4 months ago

Actually i'm not so sure I get it :disappointed:

Do you mean removing the "delayed" logic altogether? If I do that, a low frequency bar might never redraw if a high frequency bar gets all the allowances :confused:

djc commented 4 months ago

In my mind the ideal way of doing this is that we should only draw all of the multistate members together into a single drawable, relying on the MultiState::draw_target's rate limiter to decide when to draw. I think that this solution should not suffer from the problem of low frequency bars never redrawing because all bars would always get drawn at the same time.

It might be tricky to make this work out, though.