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

Improve performances during dirty paths normalization #2959

Open
wants to merge 2 commits into
base: master
from

Conversation

@Calyhre
Copy link
Contributor

commented Aug 6, 2019

Is this adding or improving a feature or fixing a bug?

Improving a feature

What's the new behavior?

Before Screen Shot 2019-08-06 at 11 52 22
After Screen Shot 2019-08-06 at 11 57 33

How does this change work?

While debugging a slow insertFragment operation, I realized Slate is spending a lot of time normalizing dirty paths and creates A LOT of intermediary arrays that are later garbage collected. Simply changing the .concat() by a .push() and actually mutate the memo instead of copying it, gives us a huge performance improvements when dealing with big chunk of operations.

Have you checked that...?

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn prettier.)
  • The relevant examples still work. (Run examples with yarn watch.)

Benchmarks:
Screen Shot 2019-08-06 at 15 58 47

Does this fix any issues or need any specific reviewers?

Fixes: #
Reviewers: @ianstormtaylor

@Calyhre Calyhre force-pushed the Calyhre:fix/applyoperation-perfs branch from 890eea5 to 0ef64d2 Aug 6, 2019

@brendancarney

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2019

We have a similar PR open here: #2559.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.