Skip to content

perf: improve rendering performance by throttling updates to 60fps#2977

Merged
MitjaBezensek merged 40 commits intomainfrom
mitja/raf
Mar 11, 2024
Merged

perf: improve rendering performance by throttling updates to 60fps#2977
MitjaBezensek merged 40 commits intomainfrom
mitja/raf

Conversation

@MitjaBezensek
Copy link
Copy Markdown
Contributor

@MitjaBezensek MitjaBezensek commented Feb 27, 2024 •
edited by steveruizok
Loading

This PR does a few things to help with performance:

  1. Instead of doing changes on raf we now do them 60 times per second. This limits the number of updates on high refresh rate screens like the iPad. With the current code this only applied to the history updates (so when you subscribed to the updates), but the next point takes this a bit futher.
  2. We now trigger react updates 60 times per second. This is a change in useValue and useStateTracking hooks.
  3. We now throttle the inputs (like the pointerMove) in state nodes. This means we batch multiple inputs and only apply them at most 60 times per second.

We had to adjust our own tests to pass after this change so I marked this as major as it might require the users of the library to do the same.

Few observations:

  • The browser calls the raf callbacks when it can. If it gets overwhelmed it will call them further and further apart. As things call down it will start calling them more frequently again. You can clearly see this in the drawing example. When fps gets to a certain level we start to get fewer updates, then fps can recover a bit. This makes the experience quite janky. The updates can be kinda ok one second (dropping frames, but consistently) and then they can completely stop and you have to let go of the mouse to make them happen again. With the new logic it seems everything is a lot more consistent.
  • We might look into variable refresh rates to prevent this overtaxing of the browser. Like when we see that the times between our updates are getting higher we could make the updates less frequent. If we then see that they are happening more often we could ramp them back up. I had an experiment for this here.

Few tests below. Used 6x slowdown for these.

Resizing

Before

CleanShot.2024-02-28.at.09.52.32.mp4

After

CleanShot.2024-02-28.at.09.53.22.mp4

Drawing

Comparison is not 100% fair, we don't store the intermediate inputs right now. That said, tick should still only produce once update so I do think we can get a sense of the differences.

Before

CleanShot.2024-02-28.at.09.54.55.mp4

After

CleanShot.2024-02-28.at.09.54.22.mp4

Change type

  • improvement

Test plan

  1. Open the drawing example and check for jankiness during high-load operations.
  • Unit tests
  • End to end tests

Release notes

  • Improves the performance of rendering by throttling updates to 60fps.

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 27, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
examples ✅ Ready (Inspect) Visit Preview Mar 11, 2024 0:22am
1 Ignored Deployment
Name Status Preview Updated (UTC)
tldraw-docs ⬜️ Ignored (Inspect) Visit Preview Mar 11, 2024 0:22am

@steveruizok
Copy link
Copy Markdown
Collaborator

steveruizok commented Mar 10, 2024

Anything left on this one?

@MitjaBezensek MitjaBezensek marked this pull request as ready for review March 11, 2024 07:07
@MitjaBezensek
Copy link
Copy Markdown
Contributor Author

MitjaBezensek commented Mar 11, 2024

Yes, it's now ready for review 😄

ds300
ds300 approved these changes Mar 11, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ds300 ds300 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I wonder if we can simplify the state chart bits by supporting an onThrottledMouseMove callback method that does the dirty checking and uses the tick under the hood, to simplify this kind of workflow that you've had to reproduce in a bunch of places? Might be more trouble than it's worth and can definitely be done separately from this PR so not blocking in any way.

packages/utils/src/lib/throttle.ts Outdated
Comment on lines +14 to +21
const tick = () => {
const queue = fpsQueue.splice(0, fpsQueue.length)
for (const fn of queue) {
fn()
}
}

function fps() {
Copy link
Copy Markdown
Collaborator

@ds300 ds300 Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extremely nonblocking nitpick:

I found the naming here a little odd. I'd personally change tick to flush and change fps to tick. For me a tick is some kind of regularly scheduled event for managing asynchronous execution of other things, which is kinda what your fps function is doing.

I dunno, could just be me though 🤷🏼

Copy link
Copy Markdown
Contributor Author

@MitjaBezensek MitjaBezensek Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed names definitely make more sense to me, thanks for the suggestion.

packages/utils/src/lib/throttle.ts
* @returns
* @internal
*/
export function throttleToNextFrame(fn: () => void) {
Copy link
Copy Markdown
Collaborator

@ds300 ds300 Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes!

throttleToNextFrame and fpsThrottle is a much less confusing naming scheme than what we had before 🙌🏼

packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts
Comment on lines +57 to +60
if (this.isDirty) {
this.isDirty = false
this.updateScribbleSelection(true)
}
Copy link
Copy Markdown
Collaborator

@ds300 ds300 Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we should run these in onComplete too? To get the absolute final positions. Probably not gonna be noticeable unless the frame rate gets quite low tbh.

This was referenced Mar 11, 2024
Closed
Merged
@MitjaBezensek MitjaBezensek mentioned this pull request Mar 19, 2024
Merged
3 tasks
@mimecuvalo mimecuvalo mentioned this pull request Jul 17, 2025
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mimecuvalo mimecuvalo mimecuvalo left review comments

@ds300 ds300 ds300 approved these changes

@steveruizok steveruizok Awaiting requested review from steveruizok

Assignees

No one assigned

Labels

improvement Product improvement major ⚙️ Increment the major version when merged sdk Affects the tldraw sdk

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MitjaBezensek @steveruizok @mimecuvalo @ds300