feat/fix github actions cache#403
Draft
Theoreticallyhugo wants to merge 4 commits into
Draft
Conversation
Theoreticallyhugo
force-pushed
the
feat/fix-github-actions-cache
branch
from
d4f5461 to
af06b52
Compare
Theoreticallyhugo
force-pushed
the
feat/fix-github-actions-cache
branch
from
af06b52 to
bcc0a18
Compare
Contributor
|
How much is the decrease in runtime? |
Contributor
Author
well, I'm still experimenting...
|
Theoreticallyhugo
force-pushed
the
feat/fix-github-actions-cache
branch
2 times, most recently
from
7800ed8 to
0d3d36f
Compare
Contributor
|
side note: pre-commit on CI seems to take 165s (see here) |
Contributor
Author
as this pr doesn't aim to change anything about pre-commit speeds, i only measure the .venv setup that comes before it. usually, setting up the .venv took about a minute, as shown here: with the changes proposed in this pr, this setup time goes down to about one second. now you might be wondering why both cicd runs took about two minutes forty for the pre-commit section. if we look closely, we can see that on one of the runs, mypy is a full minute slower than on the other. this is a separate issue that i am therefore addressing in #405. conclusion: i would commit the changes as proposed here, but maybe make one more change in the future: |
Contributor
|
Thanks a lot for the detailed breakdown, I think this is convincing. But please distill a complete PR description from your comments (guiding questions: what/why/how changes and are there side effects). |
Contributor
Author
|
I have come to realise that some of these changes don't really make sense in the grand scheme of things. limiting the package install for our cicd pipeline to those packages that are needed technically makes sense. in an ideal world we would only need the cicd tools like prek and zuban and the stubs packages that allow zuban to typecheck everything. sadly however, not all packages supply stubs, and even more sadly it's the packages with the largest dependencies. this means that we still need to install all the torch and nvidia stuff, blowing up the cicd venv. (thanks nvidia and facebook...) so, because of these select few packages that are massive but required, we either spend lots of time on building a .venv, or lots of time on mypy building the same exact venv before it checks the code. so, we now have two choices:
regardless of what we choose, this pr won't bring a big improvement in speed as hoped... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
for every pre-commit run of our github actions, we rebuild the .venv, which takes long and consumes resources. this pr tries to address that with a mix of better caching and a more sensible choice of dependencies.
main result:
pre-commit runs are about a minute faster.
details:
--only-group cicd. this removes lots of large nvidia and torch libraries and speeds up the .venv creation by about a minute. (previously it was about a minute, now its about a second)