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

Save site-packages as cache in CircleCI job #24424

Merged
merged 3 commits into from
Jun 22, 2023
Merged

Save site-packages as cache in CircleCI job #24424

merged 3 commits into from
Jun 22, 2023

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Jun 22, 2023

What does this PR do?

Currently, we save ~/.cache/pip as cache. Take check_repository_consistency job as example:

  • it installs [all, quality]
  • loading cache takes ~ 45 seconds
  • pip install takes ~ 3-4 minutes

If we save .pyenv/versions as cache too:

  • loading this new extra cache takes ~ 2 min
  • pip install takes 20 ~ 30 seconds

We gain 30 ~ 90 seconds (depending on CircleCI's states). Not a big absolute improvement. But for this job which total runtime is ~ 5m30s, we can say > 20% reduction. As check_repository_consistency and check_code_quality will be run for each PR's each push, probably it's nice to have such reduction.

WDYT?

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 22, 2023

The documentation is not available anymore as the PR was closed or merged.

@ydshieh ydshieh requested review from sgugger and amyeroberts June 22, 2023 16:25
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Do we still get updates if there is a release of one of the libraries?

@amyeroberts
Copy link
Collaborator

If we save .pyenv/versions as cache too:

loading this new extra cache takes ~ 2 min
pip install takes 20 ~ 30 seconds

@ydshieh Wait - are the timings of these in the right order?

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jun 22, 2023

Do we still get updates if there is a release of one of the libraries?

@sgugger Yes if we use -U flag. I can do that if you are ok

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jun 22, 2023

If we save .pyenv/versions as cache too:

loading this new extra cache takes ~ 2 min
pip install takes 20 ~ 30 seconds

@ydshieh Wait - are the timings of these in the right order?

Hmm, yes. But could you explain why you have doubts so I can reply in more details?

@sgugger
Copy link
Collaborator

sgugger commented Jun 22, 2023

@sgugger Yes if we use -U flag. I can do that if you are ok
YEs, please!

@amyeroberts
Copy link
Collaborator

Hmm, yes. But could you explain why you have doubts so I can reply in more details?

@ydshieh I just realised my mistake 🙃 I thought it was saying that it takes 2 mins to load with the cache and 30-40s to install by pip. Whereas it's (45 secs + 3-4 mins ) -> (2 mins + 20-30s). My bad!

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jun 22, 2023

Hmm, yes. But could you explain why you have doubts so I can reply in more details?

@ydshieh I just realised my mistake 🙃 I thought it was saying that it takes 2 mins to load with the cache and 30-40s to install by pip. Whereas it's (45 secs + 3-4 mins ) -> (2 mins + 20-30s). My bad!

The new one should be (45 secs + 2 mins + 20-30s): The first part of cache (in .cache/pip) is not changed.
But we still have a little gain overall.

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jun 22, 2023

Although already been approved - FYI: I just added -U everywhere

@ydshieh ydshieh merged commit 2c977e4 into main Jun 22, 2023
@ydshieh ydshieh deleted the run_fix_my_bad branch June 22, 2023 21:16
@ydshieh ydshieh mentioned this pull request Jul 5, 2023
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.

4 participants