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

UI: Optimize treeview render/filter performance #7910

Merged
merged 2 commits into from
Aug 29, 2019

Conversation

shilman
Copy link
Member

@shilman shilman commented Aug 29, 2019

Issue: #7907

What I did

  • Recreated a simplified version of @kevinweber's perf test inside cra-kitchen-sink
  • Optimized a few key loops in the treeview UI (w/ 2000 extra perf stories)
    • calculateTreeState (render):
      • before: ~750ms
      • after: ~10ms
    • toFiltered (search):
      • before: ~800ms
      • after: ~50ms

How to test

Edit perf.stories.js to set the outer loop max to 10 & inner to 100. Then:

cd examples/cra-kitchen-sink
yarn storybook --no-dll

@vercel
Copy link

vercel bot commented Aug 29, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@vercel vercel bot temporarily deployed to staging August 29, 2019 08:28 Inactive
@shilman shilman added this to the 5.2.0 milestone Aug 29, 2019
@vercel vercel bot temporarily deployed to staging August 29, 2019 08:55 Inactive
@vercel vercel bot temporarily deployed to staging August 29, 2019 09:28 Inactive
@shilman shilman added the bug label Aug 29, 2019
@shilman shilman merged commit 3d48116 into next Aug 29, 2019
@shilman shilman deleted the 7907-ui-perf-optimization branch August 29, 2019 16:24
@kevinweber
Copy link

Wow, I love this, such a great fix!

PS: @shilman You tagged the wrong Kevin Weber in the PR description. It should be @kevinweber (with one b)

@theKashey
Copy link
Contributor

What about increasing the limit for getParents/getParent? Nowadays it's just 1000 elements, and using array.instead instead of spread at getParents?

Now sure about the impact, but also a low hanging fruit.

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

Successfully merging this pull request may close these issues.

4 participants