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): reuse rootNode & trackedRootNodes caches across instances of graphqlRunner #33695

Merged
merged 4 commits into from
Jan 11, 2022

Conversation

KyleAMathews
Copy link
Contributor

@KyleAMathews KyleAMathews commented Oct 27, 2021

Walking nodes to track inline objects is very expensive. Caching this across
instances of the graphql query sped up subsequent queries by ~10-15% (depending on complexity of nodes).

[sc-41666]

Walking nodes to track inline objects is very expensive. Caching this across
instances of the graphql query sped up subsequent queries by ~10-15% (depending on complexity of nodes).
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 27, 2021
@KyleAMathews KyleAMathews marked this pull request as draft October 27, 2021 14:27
@KyleAMathews KyleAMathews added topic: performance Related to runtime & build performance and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 27, 2021
@KyleAMathews KyleAMathews marked this pull request as ready for review November 5, 2021 19:32
@KyleAMathews KyleAMathews changed the title feat(gatsby): pull cache out of graphql runner so can reuse feat(gatsby): pull rootNode & trackedRootNodes caches out of graphql runner so they're reused Nov 15, 2021
@KyleAMathews KyleAMathews changed the title feat(gatsby): pull rootNode & trackedRootNodes caches out of graphql runner so they're reused feat(gatsby): reuse rootNode & trackedRootNodes caches on new builds Nov 15, 2021
@KyleAMathews KyleAMathews changed the title feat(gatsby): reuse rootNode & trackedRootNodes caches on new builds feat(gatsby): reuse rootNode & trackedRootNodes caches on new instances of graphqlRunner Nov 15, 2021
@KyleAMathews KyleAMathews changed the title feat(gatsby): reuse rootNode & trackedRootNodes caches on new instances of graphqlRunner feat(gatsby): reuse rootNode & trackedRootNodes caches across instances of graphqlRunner Nov 15, 2021
@wardpeet
Copy link
Contributor

Is it possible to have a look at failing tests? 🙏


this._rootNodeMap = _rootNodeMap
this._trackedRootNodes = _trackedRootNodes
this._rootNodeMap = _rootNodeMap || new WeakMap()
Copy link
Contributor

Choose a reason for hiding this comment

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

the constructor is also called from /gatsby/packages/gatsby/src/schema/context.ts and the cache is passed there so that's why of the tests are failing. Not sure if it is ok to just create new WeakMap/WeakSet if they are not passed or the global cache can be share between the graphql-runner and the context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting. It seems generally a localNodeModal is passed in there (and perhaps always except for tests?) but yeah, this is a good fallback.

@pieh pieh changed the title feat(gatsby): reuse rootNode & trackedRootNodes caches across instances of graphqlRunner perf(gatsby): reuse rootNode & trackedRootNodes caches across instances of graphqlRunner Jan 11, 2022
@pieh pieh merged commit 26882f3 into master Jan 11, 2022
@pieh pieh deleted the cache-tracking branch January 11, 2022 12:18
moonmeister added a commit to moonmeister/gatsby that referenced this pull request Jan 11, 2022
* master: (24 commits)
  chore(docs): Release Notes v4.5 (gatsbyjs#34425)
  chore(docs): Update quick-start guide (gatsbyjs#34445)
  chore(docs) : Typo fix GatbsyImage -> GatsbyImage (gatsbyjs#34439)
  perf(gatsby): reuse rootNode & trackedRootNodes caches across instances of graphqlRunner (gatsbyjs#33695)
  Update media-item-processing.md (gatsbyjs#34434)
  chore(docs): Update localization doc (gatsbyjs#34429)
  test(ssr): Fix flakes (gatsbyjs#34443)
  chore(release): Publish next
  Revert "docs: Match egghead.io video instructions (gatsbyjs#34315)" (gatsbyjs#34384)
  fix(gatsby-plugin-manifest): generate icons sequentially (gatsbyjs#34331)
  Fix misspelling of "precedence" in log message (gatsbyjs#34428)
  chore(docs): Adjust doc mentions of gatsby-plugin-create-client-paths (gatsbyjs#34424)
  chore(docs): Update static-folder doc (gatsbyjs#34392)
  Upgrade to strip-ansi ^6.0.1 (gatsbyjs#34383)
  chore(gatsby-plugin-create-client-paths): Update client paths plugin readme with migration info (gatsbyjs#34423)
  chore: Remove deprecated client paths plugin references (gatsbyjs#34422)
  chore(docs): Old occurrences of gatbyjs.org (gatsbyjs#34402)
  Update plugins.md to have correct URL for gatsby-plugin-segment-js (gatsbyjs#34397)
  chore(gatsby): Give option to ignore output from workers and silence validate-engines (gatsbyjs#34416)
  chore(release): Publish next pre-minor
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: performance Related to runtime & build performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants