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

Fix pkgs cache on Windows #5082

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jaimergp
Copy link
Contributor

Description

conda-build's CI uses actions/cache to restore the pkgs cache conda uses to provision the environments. On Unix this takes a few seconds, but on Windows this goes all they way to 6-10mins! Apparently it's caused by tar's slowness (see actions/cache#752).

If this is to save time, I'd say it's not fitting that purpose. If it's to save Anaconda some traffic, then feel free to close.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Nov 22, 2023
@jaimergp jaimergp marked this pull request as ready for review November 23, 2023 10:12
Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

It might make more sense to use GNU tar instead to work around the performance issue: actions/cache#752 (comment)

jezdez
jezdez previously approved these changes Dec 6, 2023
Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

Actually nevermind, getsentry/sentry-dotnet#1647 seems to indicate it wasn't fixed by switching to gnu tar.

@mbargull
Copy link
Member

mbargull commented Dec 6, 2023

Ah, I forgot to comment on this one, sorry.
From what it looks like (hard to tell exactly because of fluctuating run times), it kind of cancels each other out.
Meaning, the caching takes far longer on Windows, yes, but actually the package fetching/extracting over the whole test run takes a similar amount of time.
So, to actually reduce the test duration we'd probably really have to fix/workaround the slow cache restoration (e.g., try to pack up the cache and extract manually with some archiver that works faster on Windows). 😞

@jezdez
Copy link
Member

jezdez commented Dec 7, 2023

Ah, I forgot to comment on this one, sorry. From what it looks like (hard to tell exactly because of fluctuating run times), it kind of cancels each other out. Meaning, the caching takes far longer on Windows, yes, but actually the package fetching/extracting over the whole test run takes a similar amount of time. So, to actually reduce the test duration we'd probably really have to fix/workaround the slow cache restoration (e.g., try to pack up the cache and extract manually with some archiver that works faster on Windows). 😞

https://learn.microsoft.com/en-us/virtualization/community/team-blog/2017/20171219-tar-and-curl-come-to-windows seems to indicate that bsdtar is coming to Windows builds, based on libarchive, which may fix this. We could try to install it from somewhere else?

@jezdez
Copy link
Member

jezdez commented Dec 7, 2023

I've just applied enableCrossOsArchive to test if this fixes the performance issues. Allegedly that's a feature to create cross-platform compatible caches, but also uses a single faster tar implementation to do that.

@jezdez
Copy link
Member

jezdez commented Dec 7, 2023

@jezdez
Copy link
Member

jezdez commented Dec 7, 2023

~6 minutes on Windows :-/

CleanShot 2023-12-07 at 10 14 37@2x

@jezdez
Copy link
Member

jezdez commented Dec 7, 2023

Aha! When putting conda's package dir on D: instead of C: then it's fast to unpack! Courtesy of actions/setup-go#393

CleanShot 2023-12-07 at 12 01 04@2x

~40 seconds instead of ~6 minutes!

Comment on lines 220 to 224
uses: conda-incubator/setup-miniconda@v2
uses: jezdez/setup-miniconda@windows-cache
Copy link
Member

Choose a reason for hiding this comment

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

This is a little nasty, I'll try to upstream this to setup-miniconda

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not use CONDA_PKGS_DIRS env var in the meantime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, it needs to be recorded in the config, I see.

@jezdez
Copy link
Member

jezdez commented Dec 8, 2023

Ported that to setup-miniconda: conda-incubator/setup-miniconda#328

@kenodegard kenodegard changed the title Disable pkgs cache on Windows Fix pkgs cache on Windows Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

None yet

4 participants