Skip to content

Iterative version of getFlattenedTree for improved performance on large datasets #156

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

123joshuawu
Copy link

Addresses #84

Thank you for the great library, it has worked really well and achieved amazing results with our project.

Recently, we had a performance issue with a particularly large dataset. The tree contained roughly 140k nodes with the structure being 14 nodes containing around 10k direct children each.

This would take around 1-2 minutes to render.

Quick profiling showed that getFlattenedTree was taking around 1 minute to complete.
Screen Shot 2021-04-16 at 1 08 31 PM

After changing the getFlattenedTree to an iterative implementation, the render time decreased drastically with profiling showing all getFlattenedTree calls completing within 100ms.
Screen Shot 2021-04-16 at 1 09 40 PM

Would appreciate any comments or thoughts

Copy link

@Cireo Cireo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice patch, one minor note


for (const node of nodes) {
flattenNodes.push({node, parents});
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be

const flattenNodes = nodes.map((node) => {node, parents});

@ViktorReib
Copy link

Any updates for this PR?

@Cireo
Copy link

Cireo commented Jun 9, 2021

I think maintainers aren't following PRs, last update to the repo was last September =(. Might have to fork or monkey-patch if you want it.

A little investigation on the last merged PR reveals that the owner became a father, so they probably reprioritized correctly!

@ojhawkins
Copy link

A long shot but any possibility of this being merged?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants