The Wayback Machine - https://web.archive.org/web/20220227183410/https://github.com/scikit-learn/scikit-learn/pull/22192
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

Jump to bottom

CI Removes build directory from cache #22192

Merged
merged 5 commits into from Jan 12, 2022
Merged

CI Removes build directory from cache #22192

merged 5 commits into from Jan 12, 2022

Conversation

@thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Jan 11, 2022

There were issues caching ~/project/build in #22083

This PR removes the build directory. I ran a circleci run on my fork before and after the cache, where I commented out pytest part and left the time pip install ... to check the build times.

  1. Before cache the build took 4m44.561s
  2. After loading the cache the build took 0m57.409s and we can see the cache hits:
+ ccache -s --verbose
Summary:
  Cache directory:    /home/circleci/.cache/ccache
  Primary config:     /home/circleci/.config/ccache/ccache.conf
  Secondary config:   /home/circleci/miniconda/envs/testenv/etc/ccache.conf
  Stats updated:      Tue Jan 11 19:09:21 2022
  Hits:                 72 /  154 (46.75 %)
    Direct:             72 /  156 (46.15 %)
    Preprocessed:        0 /   84 (0.00 %)
  Misses:               82
    Direct:             84
    Preprocessed:       84
  Uncacheable:         128
Primary storage:
  Hits:                144 /  312 (46.15 %)
  Misses:              168
  Cache size (GB):    0.04 / 0.00
  Files:               164
Uncacheable:
  Called for linking:  122
  Compilation failed:    2
  No input file:         4

CC @glemaitre @jjerphan @ogrisel
adrinjalali
adrinjalali approved these changes Jan 12, 2022
View changes
Copy link
Member

@adrinjalali adrinjalali left a comment

LGTM!

jjerphan
jjerphan approved these changes Jan 12, 2022
View changes
Copy link
Member

@jjerphan jjerphan left a comment

LGTM.

This is a good follow-up w.r.t https://github.com/scikit-learn/scikit-learn/pull/21174/files#r725423319.

@jjerphan jjerphan merged commit 6d7d0f2 into scikit-learn:main Jan 12, 2022
29 checks passed
@ogrisel
Copy link
Member

@ogrisel ogrisel commented Jan 12, 2022

Very nice! I am not 100% sure why restoring the build folder would prevent ccache from hitting its own cache but it's nice to see that the simplified version (without caching the build folder) works fast (thanks to ccache) and, more importantly, correctly and reliably!

@ogrisel ogrisel mentioned this pull request Jan 12, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali

@ehsanealikhani ehsanealikhani

@jjerphan jjerphan

Assignees
No one assigned
Labels
Build / CI
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
5 participants
@thomasjpfan @ogrisel @adrinjalali @ehsanealikhani @jjerphan