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: Optimize build job on CircleCI #2605

Merged
merged 5 commits into from
Oct 19, 2021
Merged

Conversation

oesteban
Copy link
Member

Pulling miniconda is a waste of time for most Circle builds, since it will only be necessary when that particular step of the docker build needs to be re-executed.

@effigies
Copy link
Member

Looks like it's getting pulled anyway.

 ---> d664ee2c3874
Step 23/44 : COPY --from=nipreps/miniconda:py38_1.3.2 /opt/conda /opt/conda
py38_1.3.2: Pulling from nipreps/miniconda

afab3874: Pulling fs layer 
6f40b3be: Pulling fs layer 
10c60a31: Pulling fs layer 
8d7a2e34: Pulling fs layer 
330ada3c: Pulling fs layer 
a77f238c: Pulling fs layer 
Digest: sha256:4d0dc0fabb794e9fe22ee468ae5f86c2c8c2b4cd9d7b7fdf0c134d9e13838729
Status: Downloaded newer image for nipreps/miniconda:py38_1.3.2
 ---> Using cache

I wonder if using the hash in the --from= bit will let it recognize that it doesn't need to fetch? e.g.

COPY --from=nipreps/miniconda@sha256:4d0dc0fabb794e9fe22ee468ae5f86c2c8c2b4cd9d7b7fdf0c134d9e13838729 /opt/conda /opt/conda

@oesteban
Copy link
Member Author

It seems the miniconda image might need to be pulled down again - anyways that is probably faster than caching.

@oesteban
Copy link
Member Author

We cross-posted - let me try the digest idea.

@effigies
Copy link
Member

Hmm. No difference.

@oesteban
Copy link
Member Author

Hmm. No difference.

I would have expected that. I need to send an empty commit and force a rebuild to be sure it's not going to pick the cached layer.

@oesteban
Copy link
Member Author

Surprisingly, the cache size of this branch is 7.1 GiB, while the fix/restore_sdc branch (which should also have nipreps/miniconda cached) is 6.2 GiB. That doesn't make any sense. We might need to fully clear-up the cache to test this.

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2021

Codecov Report

Merging #2605 (d4ba805) into master (33432c0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2605   +/-   ##
=======================================
  Coverage   45.80%   45.80%           
=======================================
  Files          41       41           
  Lines        3133     3133           
=======================================
  Hits         1435     1435           
  Misses       1698     1698           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33432c0...d4ba805. Read the comment docs.

@oesteban
Copy link
Member Author

Okay, it seems the docker image is always pulled - but it is still faster than caching it so let's merge. We are back down to ~18min for the build job. I think that, creating minimized images for FSL, FreeSurfer and AFNI will substantially reduce build time - even if we have to download or cache these images.

@oesteban oesteban merged commit 3f985d6 into master Oct 19, 2021
@oesteban oesteban deleted the docker/do-not-pull-miniconda branch October 19, 2021 07:46
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.

3 participants