Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Too many spinners causes flickering #226

Open
obsgolem opened this issue Jan 2, 2021 · 8 comments
Open

Too many spinners causes flickering #226

obsgolem opened this issue Jan 2, 2021 · 8 comments

Comments

@obsgolem
Copy link

obsgolem commented Jan 2, 2021

If you have more spinners than you have lines in your terminal viewport you get a rather large amount of flickering. This can be reduced by using set_move_cursor(true), but this removes your ability to change your spinner's message. You also lose the ability to dynamically add new spinners, and this still doesn't completely eliminate the flickering.

The example below demonstrates these effects. Change set_move_cursor to true to see how you are unable to change your spinner's message. Change set_message to tick to demonstrate that the flickering happens even when nothing but the spinner is changing on each line.

use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
use rand::Rng;
use rayon::{prelude::*, ThreadPoolBuilder};
use std::{sync::Arc, thread, time::Duration};

fn main() {
    let prog = Arc::new(MultiProgress::new());
    prog.set_move_cursor(false);

    ThreadPoolBuilder::new()
        .num_threads(40)
        .build_global()
        .unwrap();
    let prog2 = Arc::downgrade(&prog);
    thread::spawn(move || {
        while let Some(x) = prog2.upgrade() {
            x.join().unwrap();
        }
    });

    (0..40)
        .into_par_iter()
        .map(|_| {
            let style = ProgressStyle::default_spinner();
            let pb = prog.add(ProgressBar::new_spinner()).with_style(style);
            // pb.enable_steady_tick(66);
            {
                let pb = pb.clone();
                thread::spawn(move || {
                    while !pb.is_finished() {
                        let num = rand::thread_rng().gen_range(3..20);
                        pb.set_message("r".repeat(num).as_str());
                        thread::sleep(Duration::from_millis(100));
                    }
                });
            }

            let num = rand::thread_rng().gen_range(3..20);
            thread::sleep(Duration::from_secs(num));
            pb.finish_with_message("Finished");
        })
        .for_each(|_| ());
}

Some questions to ask:

  • Can you move the cursor without scrolling the terminal? This would help
  • Can you find a way to clear lines as quickly as you can move the cursor? Perhaps cache the line length, move the cursor, redraw, padding the line out to the former line length? Can you change the line clear code to skip over lines that haven't changed?
  • Can you eliminate full redraws of a line when only the spinner is changing, not the message?

Another suggestion I have is to disable the cursor, perhaps only once you have a certain number of spinners. This leads to less visible cursor flickering.

I am on Windows 10 and am testing in a regular cmd window, though I also see similar behavior in Windows Terminal.

@Bauxitedev
Copy link

Bauxitedev commented May 16, 2021

Can confirm this on my end in Windows 10. Although I've only tested using regular progress bars, not spinners. It happens in both cmd.exe and PowerShell.

Update: it doesn't seem to happen in VSCode's integrated PowerShell terminal. Weird...

@jgrund
Copy link

jgrund commented May 26, 2021

I am also hitting this, running macOS 11.3.1.

@djc
Copy link
Member

djc commented Jun 1, 2021

I tried to reproduce this on my 8-core macOS (11.4) laptop and it seems to flicker just a little bit, like 2-3 times over the 20s run of the program? Doesn't seem too bad, although I guess my laptop might be fast enough to keep up. Maybe it helps to just set the draw rate of the progress bars to something sane like 4 or 10 or something?

@alerque
Copy link

alerque commented Mar 12, 2024

I have an app that falls apart pretty quickly and starts flickering badly enough to be disturbing to watch the screen. It happens even when only a few spinners are active if there are many others that have been finished and are no longer ticking.

What you mean by "draw rate" @djc? I can try to mock up an MWE or a real live example if it would help. I've tried slowing the tick rate way down and even off, set the cursor move option, etc. but not much luck It seems like the MultiProgress is genning redrawn from the top even if only one spinner at the bottom is actually triggering redraws. Does that sound correct?

@chris-laplante
Copy link
Collaborator

I have an app that falls apart pretty quickly and starts flickering badly enough to be disturbing to watch the screen. It happens even when only a few spinners are active if there are many others that have been finished and are no longer ticking.

What you mean by "draw rate" @djc? I can try to mock up an MWE or a real live example if it would help. I've tried slowing the tick rate way down and even off, set the cursor move option, etc. but not much luck It seems like the MultiProgress is genning redrawn from the top even if only one spinner at the bottom is actually triggering redraws. Does that sound correct?

A MWE would be great! I thought MultiProgress was smart enough to not redraw things that don't need to be redrawn, but looking at the code I cannot find any evidence of that optimization :(. So it's definitely possible that's what's happening here.

@chris-laplante
Copy link
Collaborator

Maybe related to #630?

@alerque
Copy link

alerque commented Mar 14, 2024

I adapted my MWE from #636 to be useful for this issue instead of the lines overflow issue. The key difference here is that the first progress bar doesn't have a spinner here, so it shouldn't need to get updated as the other ticks happen. Only a small number of spinners should be active at once all near the end of the MultiProgress, but the MP clearly starts struggling and redrawing from the top:

#!/usr/bin/env -S cargo +nightly -Z script
## [package ]
## edition = "2021"
## [dependencies]
## indicatif = "0.17"
## lazy_static = "1.4"
## rayon = "1.9"
## console = "0.15"

use indicatif::*;
use lazy_static::lazy_static;
use std::env::args;
use std::sync::{Arc, RwLock};
use std::{thread, time::Duration};
use console::style;

lazy_static! {
    pub static ref MP: RwLock<MultiProgress> = RwLock::new(MultiProgress::new());
}

fn main() {
    // $1 is number of jobs
    let jobs: u64 = args().last().unwrap().parse().unwrap();

    // A section header that will stay above the jobs
    // NOTE: no spinner or ticks, so the top of the list of bars shouldn't need constant redraws
    let pb = ProgressBar::new_spinner()
        .with_style(ProgressStyle::with_template("{msg}").unwrap());
    let pb = MP.write().unwrap().add(pb);
    pb.set_message(style("Starting jobs").cyan().bold().to_string());

    let results = Arc::new(RwLock::new(Vec::new()));

    // A bunch of jobs with their own spinners
    rayon::scope(|s| {
        for n in 1..=jobs {
            let results = &results;
            s.spawn(move |_| {
                let pb = ProgressBar::new_spinner()
                    .with_style(ProgressStyle::with_template("{spinner} {msg}").unwrap())
                    .with_message(format!("{} number {}", style("Started job").yellow(), style(n).bold()));
                let pb = MP.write().unwrap().add(pb);
                pb.enable_steady_tick(Duration::from_millis(100));
                thread::sleep(Duration::from_millis(n * 200));
                pb.finish_with_message(format!("{} number {}", style("Ended job").green(), style(n).bold()));
                results.write().unwrap().push(true);
            });
        }
    });

    let ret = results.read().unwrap().iter().all(|&v| v);

    // Update the header at the top
    pb.finish_with_message(format!("{} = {}", style("Completed jobs").magenta().bold(), style(ret).green().bold()));

    // A trailing status message
    let pb = ProgressBar::new_spinner()
        .with_style(ProgressStyle::with_template("{msg}").unwrap());
    let pb = MP.write().unwrap().add(pb);
    pb.finish_with_message(style("Wrapped up").magenta().bold().to_string());
}

So the MWE is a single script but reproducing the scenario has a caveat. In my app this consistently acts up, but in this MWE it works fine by itself. The difference is probably that my app is burning up 100% CPU usage across all cores (on purpose of course). The progress bars are showing the status of resource intensive build tasks, so almost by definition the progress bars are only going to be used when there is minimal CPU time available.

In order to get this MWE to act up, I have to load up the system with some big compile job, then arrange a terminal to have at least 50 lines or so available and run the MWE with an argument of at least a couple lines less than the terminal height (so as not to also trigger the other bug).

@alerque
Copy link

alerque commented Mar 14, 2024

You can also adapt that MWE to use #630 by changing the indicatif dependency line:

## indicatif = { git = "https://github.com/remi-dupre/indicatif.git", branch = "fix-multibar-performance" }

I just tried it and didn't see any appreciable difference. The whole batch of ProgressBars in the MultiProgress is still getting redrawn from the top even when the last few bars are the only active ones with spinners, and the performance is still janky when the CPU is loaded up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants