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

The performance of opening the main block inserter is poor #26080

Closed
noahtallen opened this issue Oct 14, 2020 · 9 comments
Closed

The performance of opening the main block inserter is poor #26080

noahtallen opened this issue Oct 14, 2020 · 9 comments
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Performance Related to performance efforts

Comments

@noahtallen
Copy link
Member

Describe the bug
When clicking the top block inserter button, it takes about half a second to a second to finish opening. The delay is definitely noticeable. At first I noticed this in the site editor (FSE), but quickly confirmed that this is also an issue in the post editor, so it seems to be a root issue with the interester.

Here is a gif. The red circle is my click, and you can tell that it takes a while to render. It does not delay closing it though.

2020-10-13 18 02 09

I took a performance chart in chrome. The selected portion of the chart here shows just how bad it gets. It looks like for around 400ms, render performance drops to around 1 or 2fps. This performance hit is very consistent. In this graph, all the circled areas are when I click to open the block inserter. You can also see smaller troughs in between, which is where the block inserter is closing.

Screen Shot 2020-10-13 at 1 57 15 PM

cc @youknowriad if you have ideas of who might know more about this

Editor version (please complete the following information):

  • Gutenberg development repository with wp-env, up-to-date as of time of report.

Desktop (please complete the following information):

  • Browser: Firefox
  • Version: 81.0.1
@noahtallen noahtallen added [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Performance Related to performance efforts labels Oct 14, 2020
@youknowriad
Copy link
Contributor

The inserter is a piece of UI we didn't optimize much. Thanks for opening this issue, I'm pretty sure there's plenty of selector calls and rerenders we can optimize here.

@simison
Copy link
Member

simison commented Oct 14, 2020

cc @sgomes in case you'd have thoughts with your performance optimization experience!

@sgomes
Copy link
Contributor

sgomes commented Oct 14, 2020

@simison Thanks for looping me in! There is no general advice to apply to these situations; they all require in-depth analysis of what might be going on.

It's hard to tell from a screenshot of the flamechart, but the rendering delays appear to be caused by React rendering, not the browser rendering pipeline. Which means that there's likely a slow component or five that may need to be optimised.

I'll update this issue when I find some time to look into this a bit more deeply.

@david-szabo97
Copy link
Member

  • Some components are good subjects for memo (BlockIcon, Icon, ListItem)
  • Since Inserter is a long list of items, we might give virtualized list a try (see Try using virtualized list in Inserter #26107)
  • Rather than unmounting the inserter sidebar, we should keep it mounted all the time and just show-hide it. This has its own pros and cons. But together with virtualized list, this could work very well.

@noahtallen
Copy link
Member Author

Rather than unmounting the inserter sidebar, we should keep it mounted all the time and just show-hide it

I definitely think this would be a big performance improvement after first render. That was my first thought as well

@noahtallen
Copy link
Member Author

I don't have specific ideas about the virtualilzed list. But I do wonder if there is a way we can load the bocks after loading the inserter UI. Like, move when the "pause" happens to after the inserter structure is already rendered. Not sure if that would be better UX 🤔

@david-szabo97
Copy link
Member

Mounting so many new elements is a big work for React. We either reduce the DOM elements by lazy loading / virtualizing, or we mount it all the time. TBH, keeping it mounted is unavoidable and probably cheaper to keep it in DOM rather than mounting again and again every time we open it.

But keeping it in DOM is not free either, that's why we should try to reduce the DOM elements if possible. Unfortunately, looks like virtualization causes a11y problems.

Either way, I'll spin up a few experimental PRs so we can see them in action.

@Mamaduka
Copy link
Member

Mamaduka commented Aug 5, 2021

@youknowriad, can we close this after #33749 and #33868?

@youknowriad
Copy link
Contributor

Yes I think we can. there are still improvements to make but it's now responsive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Performance Related to performance efforts
Projects
None yet
Development

No branches or pull requests

6 participants