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

perf(gatsby): more efficient parent-child check through arrays #22126

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

pvdz
Copy link
Contributor

@pvdz pvdz commented Mar 10, 2020

This replaces a _.uniq with a .includes which is called N times, once for each node, while the array of nodes is building up (or on the full set even?). The only reason, I think, is to make sure the newly inserted child does not already exist in the array of children. The includes simply does a subset of things uniq does.

Another change is that a new array is no longer created; parent.children remains the same array reference after this step now.

Addresses a problem surfaced by @tsriram in #21447

Drops the source and transform step by about 33% at scale. This step in a benchmark of 100k pages drops from 30s to 20s, a 150k pages bench drops from 45s to 30s.

This is the long hanging fruit. There's still a lot of room for improvement.

@pvdz pvdz requested a review from a team as a code owner March 10, 2020 08:49
@pvdz pvdz requested a review from pieh March 10, 2020 08:49
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

LGTM!

@pvdz pvdz added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Mar 10, 2020
@gatsbybot gatsbybot merged commit be7111b into master Mar 10, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-parent-link branch March 10, 2020 09:17
@pvdz
Copy link
Contributor Author

pvdz commented Mar 10, 2020

Seems to save ~30s from the 340s total runtime on the ifsc benchmark (from 343s to 314s), 20s of that is bootstrap: https://gist.github.com/pvdz/b4eeecb062ed28a48a6d7cc16b644de9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants