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

indicatif's defaults bottleneck the application on terminal output #363

Closed
untitaker opened this issue Feb 4, 2022 · 10 comments
Closed

Comments

@untitaker
Copy link

default usage of indicatif currently has horrible performance when many items are processed in short time. there's issues like #170 which say this is due to global locks, but I found that even set_draw_rate(1000) can improve performance by an order of magnitude for certain applications.

I propose that indicatif picks better defaults for draw rate even if it's still printing to the display every millisecond, because the current default experience is broken IMO. Even if it goes as far as enabling steady tick by default to compensate for potential side effects of draw rate, I think spawning an extra thread by default would still be a preferrable default behavior over bottlenecking the entire application on terminal output.

@untitaker untitaker changed the title Pick defaults for draw rate, and maybe steady tick indicatif's settings bottleneck the application on terminal output Feb 4, 2022
@untitaker untitaker changed the title indicatif's settings bottleneck the application on terminal output indicatif's defaults bottleneck the application on terminal output Feb 4, 2022
@djc
Copy link
Collaborator

djc commented Feb 4, 2022

Have you tried this with main? There's a bunch of performance improvements.

@djc
Copy link
Collaborator

djc commented Feb 4, 2022

(I do think it might make sense to default to a draw rate of, say, 100. Want to submit a PR?)

@untitaker
Copy link
Author

untitaker commented Feb 4, 2022

I can will try main branch but I think unless the draw rate default changed or indicatif got significantly faster in tick, there's no way it would change anything

@untitaker
Copy link
Author

Very interesting, it seems that indicatif mostly removed whatever bottleneck drawing on the terminal had on master. I can still see a slight improvement with set_draw_rate instead of drawing on every update, but it's more like 10% overhead instead of 2000% overhead

@djc
Copy link
Collaborator

djc commented Feb 4, 2022

It's probably #319, or maybe #361.

@untitaker
Copy link
Author

(I do think it might make sense to default to a draw rate of, say, 100. Want to submit a PR?)

there's unresolved questions because "draw rate" currently takes precedence over "draw delta". With a default for "draw rate", draw delta... cannot be set anymore? Unsure.

@djc
Copy link
Collaborator

djc commented Feb 4, 2022

How about you make draw_rate an Option<u64> that defaults to Some(100) and is overwritten with None if the draw_delta is set? (And change the documentation accordingly.)

@djc
Copy link
Collaborator

djc commented Feb 10, 2022

I've implemented a default draw rate in 9df7ab3.

@untitaker
Copy link
Author

Thanks! Sorry, didn't get around to contributing this

@djc
Copy link
Collaborator

djc commented Feb 11, 2022

It's fine. My initial fix seems to have caused some regressions, so still investigating some follow 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

2 participants