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

convert remaining Node.get* methods to use paths #2000

Closed
1 of 14 tasks
ianstormtaylor opened this issue Jul 27, 2018 · 2 comments · Fixed by #2557
Closed
1 of 14 tasks

convert remaining Node.get* methods to use paths #2000

ianstormtaylor opened this issue Jul 27, 2018 · 2 comments · Fixed by #2557

Comments

@ianstormtaylor
Copy link
Owner

ianstormtaylor commented Jul 27, 2018

Do you want to request a feature or report a bug?

Improvement.

What's the current behavior?

In #1997 many (almost all) of the Node.get* models were converted to use paths, which are often much more performant. But a few commonly used ones remain key-based. This is unfortunate since the more often key-based getters are used, the more often the expensive to calculate key -> path mapping has to be created.

What's the expected behavior?

If we can get Slate's core internal usage of keys to be almost all gone, performance should increase a good amount.

The remaining methods that overtly take key arguments are:

  • Node.getNextBlock
  • Node.getPreviousBlock
  • Node.getOffset

But then there are also the *AtRange methods which are currently key-based under the covers as well:

  • Node.getActiveMarksAtRange
  • Node.getCharactersAtRange
  • Node.getFragmentAtRange
  • Node.getInlinesAtRangeAsArray
  • Node.getInsertMarksAtRange
  • Node.getOrderedMarksAtRange

And then some of the less standardized ones which we can either convert to using paths or try to remove entirely:

  • Node.getFirstInvalidDescendant
  • Node.getSelectionIndexes
  • Node.getOrderedMarksBetweenPositions
  • Node.getMarksAtPosition
  • Node.getTextsBetweenPositionsAsArray
@ianstormtaylor
Copy link
Owner Author

ianstormtaylor commented Jul 27, 2018

I'd go further to say that this kind of optimization (using paths instead of keys) is probably the biggest place for performance gains in the codebase, at least as far as the data model is concerned. (There may be other ones at the DOM level in slate-react.) So any help is much appreciated!

For an example of places where the key -> path map is used in ways that degrade performance, check out this comment: #1993 (comment)

@alanchrt
Copy link
Contributor

alanchrt commented Sep 7, 2018

I've been doing some profiling on large documents, and it's really looking like just improving/eliminating getSelectionIndexes alone could be a major performance win to start, depending on how much it could be improved. I'm not sure if the path-based optimization will be the ticket, but it seems likely since it does so much ancestor and descendant discovery.

screenshot from 2018-09-07 11-18-40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants