Decorations are slow

[Soreine]

We are investigating huge performance issues since we updated our app to Slate 0.33, and have noticed that using a syntax highlighting plugin, like slate-prism, slows everything a lot.

• Slate version: 0.33.3 on Chrome
• Example: visit https://gitbookio.github.io/slate-prism/ paste a non-trivial code snippet (~100 lines) and start typing inside it

I have measured our the time to React.renderToString our app on a reasonable page containing code blocks like this one. Here are the times:

• with highlighting: 16437ms to render the app and document
• without: 899ms to render the app and document

I also measured the time spent in the calls of the decorateNode of the plugin, which amounts to 188ms. So the issue most likely comes from Slate and the way decorations are applied and rendered.

[jasonphillips]

I'm also making heavy use of decorations in a project, and seeing similar performance problems that I suspect are rooted here (mainly since disabling all my decorations shows a speed-up). In my case, the decorations are injected externally upon certain events (meaning, somewhat like the search-highlighting demo here) rather than updated on each change via a decorateNode prop, which reinforces your suspicion that the slowdown is not local to calling that function.

[jasonphillips]

My very preliminary performance profiling suggested that the slowdown may be related to areDescendantsSorted (defined here) which is invoked during decoration rendering in each loop -- and which you previously suspected might cause performance problems when it was first added:
#1221 (comment)

[jasonphillips]

Even though getKeysAsArray() is memoized, the repeated scans across that large array via indexOf() could be the issue.

[jasonphillips]

So this is probably not the final / optimal solution to the problem, but may indicate a direction:

I can see that the main slowdown is simply that the react component for each child node (of a decorated code block, etc) currently receives all the decorations in its props (which may be hundreds in this case) and then iterates over all of them. What is needed is for decorations to not simply be passed to all children down the tree without discretion, and instead to be sent only to the children that overlap the decoration ranges.

Here I attempted that just on the slate-react side, by having each Node component order its decoration ranges around its children in advance and only pass down the ones that overlap each child:

master...jasonphillips:speed-up-decorations-rendering

That leads to a significant speed-up, which you should be able to see here (I merely updated the slate-prism demo page with this build):

https://jasonphillips.github.io/slate-prism/

But it's still not as fast as it could be. I'm relying on node.getKeysAsArray() for my orderings and then loop through the children, but there are surely faster ways. That being said, this is already much faster than the current practice of naively passing all decorations down the tree whether or not they fall outside the scope of each child node.

[zhujinxuan]

If the problem are caused by areDescendantsSorted, can you check whether #1771 perhaps helps a bit? The problem of areDescendantsSorts is that sorting in getKeys is very slow.

BTW, would you like to check whether #1783 helps on this problem?

[Soreine]

I'll see if I can improve upon your design @jasonphillips. As is, the performance feels good already.

I was thinking about converting the list of decorations to a sorted list of trivial decorations (a decoration with start and end on the same text key) and only pass down the decorations needed.

I'll have a look at @zhujinxuan PRs to see if there are more opportunities

[zhujinxuan]

Hi, @jacobbloom I made two small fixes in slate-react->text.js for that problem. Can you try that fix in your document and see whether that works? (If perfect, we will not worry about areSorted any one in decs)

[jacobbloom]

@zhujinxuan did you mean to pull me into this or was that an autocomplete mistake?

[Soreine]

He meant to mention @jasonphillips

FYI I have an opened PR on our fork, continuing on the work of @jasonphillips https://github.com/GitbookIO/slate/pull/3

[Soreine]

@zhujinxuan where can we see your changes?

[zhujinxuan]

Sorry, I made the wrong mention.

The change can be seen in #1771 at the slate-react-> text.js where startKey === endKey