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.

Sign up for GitHub

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

Jump to bottom

Cache documents, schema loading + os.cpus for concurrency limit + async fs #5707

Merged
merged 11 commits into from
Feb 9, 2022
Merged

Cache documents, schema loading + os.cpus for concurrency limit + async fs #5707

merged 11 commits into from
Feb 9, 2022

Conversation

kamilkisiela
Copy link
Collaborator

@kamilkisiela kamilkisiela commented Mar 14, 2021 •
edited
Loading

This should improve the performance a bit in scenarios similar to the one below.

generates:
  ./a.ts:
    schema: ./schema.graphql
    # ...
  ./b.ts:
    schema: ./schema.graphql
    # ...
  ./c.ts:
    schema: ./other-schema.graphql
    # ...

I also did

+import { cpus } from "os";

-concurrent: 4
+concurrent: cpus().length

aaand changed fs.sync to fs.async

@kamilkisiela kamilkisiela requested a review from dotansimha March 14, 2021 09:11
@changeset-bot
Copy link

changeset-bot bot commented Mar 14, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: 2cbef3b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@graphql-codegen/cli Minor
@graphql-cli/codegen Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@theguild-bot
Copy link
Collaborator

theguild-bot commented Mar 14, 2021 •
edited
Loading

The latest changes of this PR are available as alpha in npm (based on the declared changesets):

@graphql-cli/[email protected]
@graphql-codegen/[email protected]

@kamilkisiela kamilkisiela marked this pull request as ready for review March 14, 2021 09:15
@kamilkisiela kamilkisiela changed the title Cache documents and schema loading Cache documents and schema loading + os.cpus for concurrency limit Mar 14, 2021
@kamilkisiela kamilkisiela changed the title Cache documents and schema loading + os.cpus for concurrency limit Cache documents, schema loading + os.cpus for concurrency limit + async fs Mar 14, 2021
@dotansimha dotansimha force-pushed the master branch 2 times, most recently from 1a4493a to ff30197 Compare November 24, 2021 12:30
@ardatan ardatan force-pushed the master branch 2 times, most recently from 9ea19ea to 7673256 Compare November 24, 2021 23:10
@charlypoly charlypoly self-assigned this Feb 4, 2022
@vercel
Copy link

vercel bot commented Feb 9, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/theguild/graphql-code-generator/4ABHYgRAdimtAUYX6tN5mgSZ8U2z
✅ Preview: https://graphql-code-generator-git-kamil-cache-load-theguild.vercel.app

[Deployment for 2cbef3b canceled]

@theguild-bot
Copy link
Collaborator

theguild-bot commented Feb 9, 2022 •
edited
Loading

The latest changes of this PR are available as alpha in npm (based on the declared changesets):

@graphql-cli/[email protected]
@graphql-codegen/[email protected]

@charlypoly
Copy link
Contributor

Currently blocked by fs-extra, to be worked on later

cf: jprichardson/node-fs-extra#746

@charlypoly charlypoly added the status/blocked This task is blocked by another dependant task label Feb 9, 2022
@ardatan
Copy link
Collaborator

ardatan commented Feb 9, 2022

We can just go with native fs for now? @charlypoly

@charlypoly
Copy link
Contributor

We can just go with native fs for now? @charlypoly

@ardatan

yeah, @kamilkisiela told me also this, I'll refactor with native fs 👀

@charlypoly charlypoly removed the status/blocked This task is blocked by another dependant task label Feb 9, 2022
kamilkisiela
kamilkisiela commented Feb 9, 2022
View reviewed changes
Copy link
Collaborator Author

@kamilkisiela kamilkisiela left a comment

Choose a reason for hiding this comment

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

LGTM

@charlypoly charlypoly merged commit 35566a0 into master Feb 9, 2022
@kamilkisiela kamilkisiela deleted the kamil-cache-load branch February 9, 2022 14:56
BenoitRanque pushed a commit to BenoitRanque/graphql-code-generator that referenced this pull request May 29, 2022
@kamilkisiela @charlypoly @BenoitRanque
Cache documents, schema loading + os.cpus for concurrency limit + asy…
75bdf39 
…nc fs (dotansimha#5707)

* Cache documents and schema loading

* changelog

* Use os.cpus to calculate concurrency limit

* fs async

* changes

* revert config on cache (already introduced at codegen level)

* refactor(cli): use native node fs/promises

* fix(cli): imports issue

* fix(changeset): remove useless changeset

* fix(cli): remove duplicated code..

* fix(cli): remove useless async/await

Co-authored-by: Charly POLY <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ardatan ardatan ardatan approved these changes

@charlypoly charlypoly charlypoly approved these changes

@dotansimha dotansimha Awaiting requested review from dotansimha

Assignees

@charlypoly charlypoly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kamilkisiela @theguild-bot @charlypoly @ardatan