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

fix(gatsby): merge data deps state instead of replaying actions #32440

Merged
merged 10 commits into from
Jul 23, 2021

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Jul 20, 2021

Description

This PR is a follow-up to #32305 After benchmarking on a heavy site we've learned that replaying actions for data dependencies is expensive. So basically this PR implements solution described in #32305 (comment):

If data dependencies will be too expensive to sync this way - we can do a hybrid approach - forward/replay some actions and merge expensive parts all at once in the end.

Now we merge the data dependency state at the end of the build. It is still expensive but far less expensive than replaying all data dependency actions: 21s vs 250s for this heavy test site:

success run queries in workers - 378.129s - 25789/25789 68.20/s
success Merge worker state - 21.678s

But query state files are huge, so maybe this is not a typical scenario (requires additional research):

-rw-r--r--  1 root root 320M Jul 20 15:38 redux.worker.slices_1_5d05d7d9a7164ada5d792186cc7691ab
-rw-r--r--  1 root root 306M Jul 20 15:38 redux.worker.slices_2_5d05d7d9a7164ada5d792186cc7691ab
-rw-r--r--  1 root root 317M Jul 20 15:38 redux.worker.slices_3_5d05d7d9a7164ada5d792186cc7691ab

[ch34739]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 20, 2021
@vladar vladar removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 20, 2021
@vladar vladar added the topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) label Jul 20, 2021
@LekoArts LekoArts marked this pull request as ready for review July 22, 2021 08:11
pieh
pieh previously approved these changes Jul 23, 2021
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.

Looks good. My comments are not blocking and PR can be merged without addressing them (they could be addressed in follow up or just skipped completely) - up to you :)

@LekoArts LekoArts added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jul 23, 2021
@gatsbybot gatsbybot merged commit fd1d8cc into master Jul 23, 2021
@gatsbybot gatsbybot deleted the vladar/merge-data-deps-state branch July 23, 2021 10:05
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 topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants