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

chore(ci): disable node cache to speed up ci #6939

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

boredland
Copy link
Contributor

@boredland boredland commented Mar 13, 2023

☕️ Reasoning

in my experience restoring and storing cache costs so much time, that the (tiny if even) gain during install is gone. this tries to prove that for next-auth.

before:

install: 1m 16s
post-setup: 1m 44s

after:

install: 1m 11s
post-setup: 0s

... also yamllint complaint about redundant string indicators.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

in my experience restoring and storing cache costs so much time, that
the (tiny) gain during install is gone. this tries to prove that for
next-auth.
@vercel
Copy link

vercel bot commented Mar 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
auth-docs ❌ Failed (Inspect) Mar 13, 2023 at 9:56PM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
next-auth-docs ⬜️ Ignored (Inspect) Mar 13, 2023 at 9:56PM (UTC)

@boredland boredland changed the title ci: disable node cache to make ci faster ci: disable node cache to speed up ci Mar 13, 2023
@boredland boredland marked this pull request as ready for review March 13, 2023 22:05
Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Interesting observation. We should have a deeper look into our CI at some point. We often have cache misses for the docs build as well.

We can merge this for now and see. Thanks!

@balazsorban44 balazsorban44 changed the title ci: disable node cache to speed up ci chore(ci): disable node cache to speed up ci Mar 14, 2023
@balazsorban44 balazsorban44 merged commit b278975 into nextauthjs:main Mar 14, 2023
@AlexErrant
Copy link

AlexErrant commented Apr 22, 2023

In your "before", the workflow file says cache: "pnpm", which is great, but the run does not use the cache: "pnpm cache is not found". This is probably because the package.json or lockfile changed and no cache for that package.json/lock file exists. After this run however, it does exist, so it should be available for subsequent runs and may improve their times. You may still be correct in that restoring from a cache isn't as fast as downloading from scratch, but your before does not demonstrate this.

In my own project (not a fork of NextAuth), caching is faster. Here's a screenshot of my own project showing how the cache improves overall pnpm i times (n=1):

image

Left. Right.

@SauravMaheshkar
Copy link
Contributor

In your "before", the workflow file says cache: "pnpm", which is great, but the run does not use the cache: "pnpm cache is not found". This is probably because the package.json or lockfile changed and no cache for that package.json/lock file exists. After this run however, it does exist, so it should be available for subsequent runs and may improve their times. You may still be correct in that restoring from a cache isn't as fast as downloading from scratch, but your before does not demonstrate this.

In my own project (not a fork of NextAuth), caching is faster. Here's a screenshot of my own project showing how the cache improves overall pnpm i times (n=1):

image

Left. Right.

@balazsorban44 thoughts on this 👀 ?

@AlexErrant
Copy link

Gonna guess he doesn't care that much about a CI speedup. If you care, you could fork, run it in your own repo, and come back with empirical runtimes.

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.

None yet

4 participants