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

Throttle updates to N FPS (e.g. 60)? #212

Closed
KyleAMathews opened this issue Jul 8, 2019 · 5 comments
Closed

Throttle updates to N FPS (e.g. 60)? #212

KyleAMathews opened this issue Jul 8, 2019 · 5 comments

Comments

@KyleAMathews
Copy link

Given Ink has to update the entire screen — it might be nice to throttle updates globally to e.g. 60 FPS. We're seeing some issues in Gatsby like gatsbyjs/gatsby#15505 that we can fix case by case but a global render throttle seems useful to explore.

@vadimdemedes
Copy link
Owner

Agree, would be a good improvement.

@vadimdemedes
Copy link
Owner

vadimdemedes commented Sep 26, 2019

Released 2.4.0, which throttles rendering to 60 FPS, if experimental mode is enabled. Thanks for suggestion!

Will keep this issue open until new reconciler and renderer becomes a default one.

pvdz added a commit to gatsbyjs/gatsby that referenced this issue Nov 28, 2019
…his way

This change explicitly throttles the progress bar updates (`tick` and `total`) to be clamped to 10 fps max.

This drops the govbook benchmark from 210 seconds down to 140 seconds.

Fixes #15505
Fixes #17452
Fixes #17966
Fixes #18801

Relates to #17873
Relates to vadimdemedes/ink#212
gatsbybot pushed a commit to gatsbyjs/gatsby that referenced this issue Nov 28, 2019
…19866)

* perf(gatsby-cli): throttle progress bar, build significantly faster this way

This change explicitly throttles the progress bar updates (`tick` and `total`) to be clamped to 10 fps max.

This drops the govbook benchmark from 210 seconds down to 140 seconds.

Fixes #15505
Fixes #17452
Fixes #17966
Fixes #18801

Relates to #17873
Relates to vadimdemedes/ink#212

* Force flush on done() and use local var for total
gatsbybot added a commit to gatsbyjs/gatsby-starter-blog that referenced this issue Nov 28, 2019
…#19866)

* perf(gatsby-cli): throttle progress bar, build significantly faster this way

This change explicitly throttles the progress bar updates (`tick` and `total`) to be clamped to 10 fps max.

This drops the govbook benchmark from 210 seconds down to 140 seconds.

Fixes #15505
Fixes #17452
Fixes #17966
Fixes #18801

Relates to #17873
Relates to vadimdemedes/ink#212

* Force flush on done() and use local var for total
@eps1lon
Copy link

eps1lon commented Apr 1, 2020

I gave it a try and it definitely helped (70s runtime instead of 240s). Though it's still faster to manually batch updates at 30fps (40s instead of 70s).

It would be perfect if we could set the fps ourselves. 60fps is too much IMO as a default. 60fps is needed for smooth animations and I'd say that most CLIs don't need smooth animations.

@vadimdemedes
Copy link
Owner

Yeah, good suggestion. I'll try lowering it to 30fps. I'm not sure ability to customize fps would be a good move, because it's more of avoiding the root cause of a problem, rather than properly fixing it. For example, a progress bar component might be implemented to refresh every 100ms, when 1s would be more than enough.

One thing I wanted to do for a while that could help performance a lot is avoiding rerendering layout when underlying React components didn't change (#21).

@vadimdemedes
Copy link
Owner

vadimdemedes commented Jul 27, 2020

Ink 3 is out with the fix for this issue included! Read the full announcement at https://vadimdemedes.com/posts/ink-3 :)

leonhiat added a commit to leonhiat/gatsby-starter-blog that referenced this issue Oct 31, 2023
…#19866)

* perf(gatsby-cli): throttle progress bar, build significantly faster this way

This change explicitly throttles the progress bar updates (`tick` and `total`) to be clamped to 10 fps max.

This drops the govbook benchmark from 210 seconds down to 140 seconds.

Fixes #15505
Fixes #17452
Fixes #17966
Fixes #18801

Relates to #17873
Relates to vadimdemedes/ink#212

* Force flush on done() and use local var for total
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants