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): use fastq instead of better-queue + refactor #31269

Merged
merged 3 commits into from
May 6, 2021

Conversation

vladar
Copy link
Contributor

@vladar vladar commented May 5, 2021

Description

This PR is a polished version of this commit: 78d6986

Original comment from @KyleAMathews:

Various optimizations to query running that make QPS 20% faster. I moved all work into the queue so the event loop wouldn't get overwhelmed for large sites + moved to a more efficient queue https://www.npmjs.com/package/fastq — our previous queue, better-queue was ~7-10% of the CPU by itself and fastq is maybe 0.1%.

One major benefit of better-queue was support for assigning priority to tasks. We used it in develop to run the currently active query first. But after "query on demand" this is no longer needed as we run only active queries anyway.

Benchmarks

Benchmarks with query-filter-sort site:

cross-env NUM_NODES=10000 yarn bench

Before:

success run page queries - 1.962s - 1001/1001 510.13/s
success run page queries - 2.088s - 1001/1001 479.45/s
success run page queries - 1.735s - 1001/1001 576.95/s
success run page queries - 2.056s - 1001/1001 486.87/s
success run page queries - 1.780s - 1001/1001 562.41/s
success run page queries - 1.853s - 1001/1001 540.20/s
success run page queries - 1.676s - 1001/1001 597.24/s
success run page queries - 1.740s - 1001/1001 575.15/s

After:

success run page queries - 1.573s - 1001/1001 636.54/s
success run page queries - 1.566s - 1001/1001 639.17/s
success run page queries - 1.637s - 1001/1001 611.55/s
success run page queries - 1.627s - 1001/1001 615.29/s
success run page queries - 1.665s - 1001/1001 601.12/s
success run page queries - 1.763s - 1001/1001 567.75/s
success run page queries - 1.566s - 1001/1001 639.24/s
success run page queries - 1.575s - 1001/1001 635.54/s
success run page queries - 1.553s - 1001/1001 644.61/s

Interestingly version of fastq with promises is slower (has basically the same numbers as current version)

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 5, 2021
@vladar vladar added topic: scaling builds and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 5, 2021
@vladar vladar changed the title perf(gatsby): refactor query queueing and use fastq instead of better-queue perf(gatsby): use fastq instead of better-queue + refactor May 5, 2021
@KyleAMathews
Copy link
Contributor

Good deleted/added code ratio 👍

Looks like a ~12% speedup for 1000 queries (using my handy one-liner — split(\n).map(l => parseFloat(l.split( ).slice(-1)[0].split(/)[0])).reduce((a,b) => a + b) / 9)

I got 20% but that was with 100k queries — the more queries the bigger the speedup I'd assume given better-queue's sorting logic would get slower on bigger datasets.

@vladar
Copy link
Contributor Author

vladar commented May 5, 2021

Yeah, also benchmark setup is probably different. I believe my benchmark still does a pass of fast filters (since I am not filtering by the id field). Filtering by id has less overhead and so the perf benefit is more noticeable.

@vladar vladar marked this pull request as ready for review May 6, 2021 08:27
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.

Awesome results. LGTM

@pieh pieh merged commit fef8d6b into master May 6, 2021
@pieh pieh deleted the vladar/fastq-query branch May 6, 2021 10:46
moonmeister added a commit to moonmeister/gatsby that referenced this pull request May 7, 2021
* master: (45 commits)
  chore(release): Publish next pre-minor
  fix(gatsby-source-shopify): fix linting (gatsbyjs#31291)
  fix(deps): update minor and patch for gatsby-plugin-preact (gatsbyjs#31169)
  chore: add gatsby-plugin-gatsby-cloud to renovate
  chore: update renovatebot config to support more packages (gatsbyjs#31289)
  chore(deps): update dependency @types/semver to ^7.3.5 (gatsbyjs#31148)
  fix(deps): update minor and patch for gatsby-plugin-manifest (gatsbyjs#31160)
  fix(deps): update minor and patch for gatsby-remark-copy-linked-files (gatsbyjs#31163)
  fix(deps): update dependency mini-css-extract-plugin to v1.6.0 (gatsbyjs#31158)
  chore(deps): update dependency @testing-library/react to ^11.2.6 (gatsbyjs#31168)
  docs(gatsby-source-shopify): Updates Shopify README with new plugin info (gatsbyjs#31287)
  chore: run yarn deduplicate (gatsbyjs#31285)
  docs(gatsby-plugin-image): Add docs for customizing default options (gatsbyjs#30344)
  fix(gatsby-plugin-image): print error details (gatsbyjs#30417)
  chore(docs): Update "Adding Search with Algolia" guide (gatsbyjs#29460)
  chore(docs): Update MDX frontmatter for programmatic pages (gatsbyjs#29798)
  docs: Add image plugin architecture doc (gatsbyjs#31096)
  perf(gatsby): use fastq instead of better-queue + refactor (gatsbyjs#31269)
  feat(gatsby-plugin-image): Export ImageDataLike type (gatsbyjs#30590)
  fix(gatsby): update plugin api types (gatsbyjs#30819)
  ...
axe312ger pushed a commit that referenced this pull request May 20, 2021
* perf(gatsby): refactor query queueing and use fastq instead of better-queue

* restore graphql-runner

* remove "better-queue" dependency
This was referenced Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants