-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Enable ccache in travis #6370
Enable ccache in travis #6370
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a line here needs to be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one line here needs to be remove. And sorry for the noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have numbers on how much time will be saved for build tests?
I only helps once you have a cache, but for the |
😢 I hoped more, but a min is better than nothing |
Restarted CI. |
@junghans Recently I had to work on a C++ project. Bottomline, I had a much higher cache hit rate by adding: addons:
apt:
sources:
- sourceline: 'ppa:likemartinma/devel'
packages:
- ccache
- ...
cache:
ccache: true
...
before_install:
- ccache -M 2G
... which updates |
Fine with me, ping @tgamblin @scheibelp |
2e0cffc
to
b85b65a
Compare
@adamjstewart all added. |
ping @scheibelp |
I'll be able to look at this by tomorrow (8/1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions:
- Enable ccache in travis #6370 (comment) adds
ccache: true
to the.travis.yml
file, is that not necessary? (I don't see it in the diff) - How are the spack builds making use of this if
ccache
is off by default in the spack config (based on ccache support #3761)?
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then virtualenv venv; fi | ||
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then source venv/bin/activate; fi | ||
- ccache -M 2G && ccache -z |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this move after the next line on osx (otherwise, how is ccache found there?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the line below it to add the compiler wrappers to the path, ccache
is already in the path.
@scheibelp, added To answer your other question, the ccache compiler wrapper are used by spack, so spack doesn't know it is ccached. |
Sorry but I'm not clear how that is coming about: is ccache placing a wrapper in /usr/bin/gcc (for example, or otherwise presenting itself as the compiler)? |
It places a wrapper in |
I think I get it now. I wish this took advantage of #3761 to get spack to use ccache explicitly. If ccache creates wrappers for all instances of gcc on I think that would be as simple as creating
|
Thanks! However: it looks like you updated |
Doh! |
@scheibelp Better now? |
This seems very clear to me so thanks for that update. However now I'm looking at |
Closed and reopened to trigger a re-build |
@adamjstewart I was wondering if this could be beneficial to avoid Travis erroring out when it builds |
@junghans Can you push any change, or rebase on develop, to see how many cache hits do we get? |
Yeah, if this helps with the mpich builds, that would be great! |
7e98ae2
to
ec7dc6c
Compare
@alalazo Done |
@alalazo: search for |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
caching seems to work again. |
@junghans I give up: I think I restarted the |
@junghans I am not strongly opposed to #10105 is an alternative solution that does not use EDIT: Travis failed with a 20 min. time-out 😭 |
cf34c69
to
1fc3b4f
Compare
@alalazo, rebased and works! |
Great! @tgamblin Can we merge this? |
@tgamblin PR merged |
This is save time in build tests phase