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

Thoughts on RichText & Performance #15033

Closed
ellatrix opened this issue Apr 18, 2019 · 4 comments
Closed

Thoughts on RichText & Performance #15033

ellatrix opened this issue Apr 18, 2019 · 4 comments
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@ellatrix
Copy link
Member

While trying to make the native macOS color button work (found in the Touch Bar when using Gutenberg in Safari), I stumbled upon a performance issue: when sliding the control, the response time is so long that you can see the active color indicator slowly moving from one side to another. The problem is that Safari just emits an input event for every color in the range control you slide through, and Gutenberg cannot keep up with updating the store and rerendering everything.

This could be solved by temporarily not calling the onChange prop for certain changes like color formatting, so the change still happens inside RichText, but the store update is delayed. This made me wonder if we cannot simple do the same for any change, as it might benefit fast typing performance as well. I recall something similar being discussed a while ago, before #13056 came.

The other option is to look into to again look into reducing the work done after onChange is called. I don't know where to even start with this. Cc @aduth @youknowriad.

Why do we need rich text changes to be in the store immediately?

  • We all agree it's the ideal.
  • We might need it later for something like smooth collaborative editing.
  • ?

Other options:

There are two things that could improve:

  • Constant input event emission, where even a 100 ms buffer would help.
  • Fast-normal typing, let's say > 40 WPM. Let's say the average word in a document is 5 characters. Then that's 200 characters per minute, or 3.33 characters per second, which means a character is added every 300 ms. For that to improve you'd need a buffer of at least twice that time?

For both to improve, it might be good to only call onChange whenever an undo level is added. This is currently done the second after the user stops typing. Would it be a bad thing to only sync the value at undo intervals? This might also remove some complexities in the store around persistence?

I know something similar has been proposed before by @youknowriad.

@ellatrix ellatrix added the [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. label Apr 18, 2019
@aduth
Copy link
Member

aduth commented Apr 18, 2019

Historically, my hesitations around delayed synchronization is the inevitable possibilities for race conditions. I think it would also be quite difficult to account for in end-to-end tests without introducing huge unreliability. These are solvable problems, but will be an ongoing maintenance burden I suspect. Synchronous updates provide a nice comfort in the certainty, and in ways force us to be mindful of the performance impact.

That said, reflecting on AsyncMode in #13056, I think it was a huge success in leveraging lazy updates to dramatically improve overall feel for users.

The other option is to look into to again look into reducing the work done after onChange is called. I don't know where to even start with this. Cc @aduth @youknowriad.

Is this specifically referring to the distinction of the onInput and onChange props of BlockEditorProvider? The worry I'd have there is that the change prop is currently used exclusively in the context of considering changes for undo history. Actual changes to values occur in onInput. Without responding to onInput, the UI wouldn't necessarily be accurate in response to user updates, unless each individual component would manage its own state in this way (similar to what you're talking about with RichText).

@ellatrix
Copy link
Member Author

Is this specifically referring to the distinction of the onInput and onChange props of BlockEditorProvider?

@aduth No, I'm talking about all the work that happens from the moment RichText calls its onChange prop: updating the store, components calling all their selectors, etc. I'm not sure what we can improve there.

Perhaps we can do something like AsyncMode in #13056, but instead of just the page AND the selected block re-rendering (correct me if I'm wrong), re-render only the selected RichText instance, and the rest of the page and block lazily.

@youknowriad youknowriad added [Type] Discussion For issues that are high-level and not yet ready to implement. and removed [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Jun 4, 2020
@mtias mtias added the [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable label Aug 30, 2020
@kbrown9
Copy link
Contributor

kbrown9 commented Jan 17, 2022

Hi! I think the suggestions for improving RichText performance could help with an issue that I'm seeing with a significant lag in resizing blocks while typing on mobile devices (see my comment here). I have seen this problem occur on both older and newer mobile devices.

I spent a little bit of time looking into the cause, and it does seem to be the amount of work that's done in the onChange prop.

@youknowriad
Copy link
Contributor

I think since this issue was opened, we made a lot of progress to typing performance and performance in general. Maybe it's time to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests

5 participants