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

Better ETA estimation for long running benchmarks #670

Open
martinus opened this issue Jul 27, 2023 · 6 comments
Open

Better ETA estimation for long running benchmarks #670

martinus opened this issue Jul 27, 2023 · 6 comments
Labels

Comments

@martinus
Copy link

I'm currently using hyperfine to benchmark something where a single benchmark run takes about 2 hours. It would be nice if the ETA would updated more often (say once every second) with a new estimate

@sharkdp
Copy link
Owner

sharkdp commented Aug 21, 2023

It would be nice if the ETA would updated more often (say once every second) with a new estimate

This should actually be the case? Or would you expect any update on the initial timing run?

@sharkdp
Copy link
Owner

sharkdp commented Aug 21, 2023

Actually, something seems wrong with the ETA estimates at the moment. If I run e.g.

hyperfine --runs=2 "sleep 5"

The ETA shows 5 seconds after the initial timing run (which is expected). But then the ETA time starts increasing... instead of decreasing.

I'm pretty sure this was working correctly in the past. This seems like a regression.

(note that ETA estimates are done by the "indicatif" dependency, not by hyperfine directly. but maybe we are using it incorrectly.)

@sharkdp sharkdp added the bug label Aug 21, 2023
@martinus
Copy link
Author

martinus commented Aug 21, 2023

I did a git bisect, it seems in 2cd2773 the strange runaway time was introduced, most likely when indicatif was upgraded from 0.17.4 to 0.17.5

Before that, it was working correctly but doesn't update as often as I'd like to.

hyperfine "sleep 5"

only shows a new ETA after a run is complete, so after the first run it jumps to 5 seconds, and stays at 5 seconds until it has finished. That's ok for 5 seconds, but when the benchmark takes 2 hours that means the ETA will be stuck at 2 hours until it's actually finished

@sharkdp
Copy link
Owner

sharkdp commented Aug 21, 2023

only shows a new ETA after a run is complete, so after the first run it jumps to 5 seconds, and stays at 5 seconds until it has finished. That's ok for 5 seconds, but when the benchmark takes 2 hours that means the ETA will be stuck at 2 hours until it's actually finished

Yeah, this is not great. I agree. I would expect indicatif to handle this. We are updating the ETA estimate every few milliseconds.

I did a git bisect, it seems in 2cd2773 the strange runaway time was introduced, most likely when indicatif was upgraded from 0.17.4 to 0.17.5

Thanks! I can actually reproduce the bug using a very simple program (indicatif 0.17.6):

use std::thread;
use std::time::Duration;

use indicatif::{ProgressBar, ProgressStyle};

fn main() {
    let pb = ProgressBar::new(2);
    pb.enable_steady_tick(Duration::from_millis(100));
    pb.set_style(
        ProgressStyle::default_spinner()
            .template("{wide_bar} ETA {eta_precise}")
            .unwrap(),
    );
    for _ in 0..2 {
        thread::sleep(Duration::from_millis(5000));
        pb.inc(1);
    }
    pb.finish_and_clear();
}

This shows the same problem (ETA jumps to 5 sec — which is fine, then increases). I will report this upstream, unless you want to do that?

@martinus
Copy link
Author

I can't actually program in rust, I'd prefer if you can do that :)

@sharkdp
Copy link
Owner

sharkdp commented Aug 21, 2023

console-rs/indicatif#580

sharkdp added a commit that referenced this issue Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants