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 perf delaying conversion to immutable objects #794

Merged
merged 1 commit into from
May 5, 2017

Conversation

ianstormtaylor
Copy link
Owner

A few performance tweaks to help ease some of the issues with #791...

Looking in the Performance tab of devtools, we can see that most of the tab is spent in re-rendering the toolbar buttons. Because when the selection changes, they need to recalculate whether they should be "active" or not.

image

Which (on my machine), when selecting the "large document" amount of text in the "rich text" example is taking 38 seconds. 😱

Most of the time is spent re-rendering the mark buttons...

image

But there is also a decent chunk spent recalculating the block buttons...

image

image

All in all the largest amount of time is spent in...

78.2% RichText.renderToolbar()
    64.5% RichText.renderMarkButton()
        60.8% RichText.hasMark()
            60.8% getMarksAtRange()
                60.5% getCharactersAtRange()
                    60.5% reduce()
                        59.2% concat()
                            -> Immutable.js internals
    13.6% RichText.renderBlockButton()
        12.9% RichText.hasBlock()
            12.8% getBlocksAtRange()
                9.8% getClosestBlock()
                    9.6% getAncestors()
                        3.6% hasChild()
                            2.0% getChild()
                3.0% toOrderedSet()
                    -> Immutable.js internals

This PR changes the way lots of the get* and get*AtRange methods work to delay converting to Immutable.js objects until after iterating, since it's a lot faster to use .map, .filter, etc. with regular arrays when possible than with immutable objects.

After all of the changes, the benchmarks show a pretty decent increase for many things...

image

And now re-running the same performance analysis shows that the total time for the same re-rendering is now ~8 seconds instead of 38. And now the biggest time sink is actually in the block buttons, because the mark buttons have been significantly sped up...

image

Still more work to be done, but a pretty big win for very little work.

@ianstormtaylor ianstormtaylor merged commit d7fa54a into master May 5, 2017
oyeanuj added a commit to oyeanuj/slate that referenced this pull request May 7, 2017
* ianstormtaylor/master: (21 commits)
  Fix json (ianstormtaylor#798)
  update readme
  0.19.29
  allow memoization without arguments for faster lookups (ianstormtaylor#796)
  0.19.28
  Add tests for plugins (ianstormtaylor#792)
  improve perf delaying conversion to immutable objects (ianstormtaylor#794)
  0.19.27
  fix schema normalizing to merge into history
  0.19.26
  `moveNode` operation fix (ianstormtaylor#782)
  fix large document example imports
  refactor examples, upgrade dependencies
  v0.19.25
  fix void copying to attach to the right dom element
  0.19.24
  0.19.23
  Add firstOnly prop to Placeholder (ianstormtaylor#765)
  rename issue template
  Copy void (ianstormtaylor#788)
  ...
@ianstormtaylor ianstormtaylor deleted the fix-perf-as-array branch May 24, 2017 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant