simplify ccache integration #182

mathias-luedtke · 2017-06-18T13:29:13Z

This PR simplifies the use of ccache as reported in #181
It is not enabled by default because it needs set-up in the CI config.

Example config for travis:

language: generic
services:
  - docker

cache:
  directories:
    - $HOME/.ccache # can be any valid cache location

env:
  global:
    - CCACHE_DIR=$HOME/.ccache # should match the cached directory above
  matrix:
    - ROS_DISTRO="kinetic" ROS_REPO=ros

install:
  - git clone --depth=1 https://github.com/ros-industrial/industrial_ci.git .ci_config
script: 
- .ci_config/travis.sh

A test-case can be added on top of #169

mathias-luedtke · 2017-06-18T17:41:41Z

I have run some tests with #169

Without cache:
Total time 2 hrs 21 min 25 sec, Ran for 47 min 57 sec
With cache:
Total time 2 hrs 6 min 41 sec, Ran for 33 min 47 sec

130s

Nice feature!

I pushed a commit for more documentation. Please review if everything I've written is correct.

LGTM. Only have minor questions as follows and an inline comment.

Beginner question; the feature in this PR is different from what Travis CI offers and is not dependent on it, correct? Just making sure because I think we don't want to Travis-specific.

Just looking at how long/short CI took,

A build with larger number of jobs seems fantastic simplify ccache integration #182 (comment)
For smaller number of jobs, e.g. ros_canopen, any idea why the total amount of time actually increased?:
- without this PR, https://travis-ci.org/ipa-mdl/ros_canopen/builds/236271434 took 33 mins.
- with this PR, https://travis-ci.org/ipa-mdl/ros_canopen/builds/244209632 took 37 mins.

Do we want to use this feature for an existing job or add a new job?

130s · 2017-06-23T01:31:57Z

industrial_ci/src/tests/source_tests.sh

+if [ "$CCACHE_DIR" ]; then
+    ici_time_start setup_ccache
+    sudo apt-get install -qq -y ccache || error "Could not install ccache. Exiting."
+    export PATH="/usr/lib/ccache:$PATH"


So this line looks cumbersome, but we may just need it https://askubuntu.com/questions/470545/how-do-i-set-up-ccache (I found @VictorLamoine there :))?

Using symlink is the recommended way to enable ccache.
On debian/ubuntu these symlinks get set-up automatically in /usr/lib/ccache by update-ccache-symlinks

mathias-luedtke · 2017-06-23T08:17:24Z

Beginner question; the feature in this PR is different from what Travis CI offers and is not dependent on it, correct? Just making sure because I think we don't want to Travis-specific.

It depends on the CI system to provide caching between the jobs.
Travis supports it, Gitlab supports it as well.
But they differ in location restrictions for caching.

For smaller number of jobs, e.g. ros_canopen, any idea why the total amount of time actually increased?:

The handling of the cache takes time.

Do we want to use this feature for an existing job or add a new job?

Travis supports only global caching (?), so the handling is done for all jobs, even if the cache is not used in the job.
However, this is just an annoyance for our tests, but it should not affect our users.

130s · 2017-06-23T14:36:11Z

Any comment on the document I updated?

Beginner question; the feature in this PR is different from what Travis CI offers and is not dependent on it, correct? Just making sure because I think we don't want to Travis-specific.

It depends on the CI system to provide caching between the jobs.
Travis supports it, Gitlab supports it as well.
But they differ in location restrictions for caching.

I see. This means that my document update was based on my misunderstanding, so definitely needs review.

For smaller number of jobs, e.g. ros_canopen, any idea why the total amount of time actually increased?:

The handling of the cache takes time.

Dumb question but do you mean caching may not work for the purpose for builds with a) smaller number of jobs b) jobs that take shorter c) else?

Do we want to use this feature for an existing job or add a new job?

Travis supports only global caching (?), so the handling is done for all jobs, even if the cache is not used in the job.
However, this is just an annoyance for our tests, but it should not affect our users.

So...do we want to test this feature in our own build (i.e. do we want to enable this feature .travis.yml on industrial_ci) or not?

mathias-luedtke · 2017-06-23T15:29:39Z

Any comment on the document I updated?

Not yet, I will have a look as soon as I have some time.

The handling of the cache takes time.

Dumb question but do you mean caching may not work for the purpose for builds with a) smaller number of jobs b) jobs that take shorter c) else?

The cache feature (of travis) downloads and unzips an archive before every build.
And after each build, the content gets compared and uploaded if something has changed.
In addtion compiling with ccache can be slower because of the overhead it introduces.
So, caching with ccache only makes sense if there are many files to compile.

In https://travis-ci.org/ipa-mdl/ros_canopen/builds/244209632 only job 4 uses ccache to speed-up the build.
The other jobs just have the Travis-cache handling overhead.

So...do we want to test this feature in our own build (i.e. do we want to enable this feature .travis.yml on industrial_ci) or not?

I did not plan to add it in this PR, but perhaps in #169
But we could give it a try to see what happens

mathias-luedtke · 2017-06-24T09:34:00Z

doc/index.rst

@@ -249,6 +250,24 @@ Reference:
 * `Discussion about install space <https://github.com/ros-industrial/industrial_ci/issues/54>`_
 * `Detail for catkin config <http://catkin-tools.readthedocs.io/en/latest/verbs/catkin_config.html>`_ for more info about `catkin-tools`.

+Cache build artifacts to speed up the subsequent builds (if any)
+----------------------------------------------------------------
+


Instructions for setting up the CI cache are missing

mathias-luedtke · 2017-06-24T09:34:55Z

doc/index.rst

+
+  env:
+    global:
+      - CCACHE_DIR="$HOME/.ccache"


I guess the quotes must be removed in order to allow variable substitution.

mathias-luedtke · 2017-06-25T14:08:36Z

The cache feature (of travis) downloads and unzips an archive before every build.
And after each build, the content gets compared and uploaded if something has changed.

This is basically true, but all jobs get individual caches, so the cache should be empty if it wasn't used. So the overhead is more or less neglectable.

But we could give it a try to see what happens

I have run some tests, and for the current tests we dont gain any speed-up with ccache because we don't build code in most jobs,
In addition I don't have an idea how we can test that the cache feature was actually run..

VictorLamoine · 2017-06-26T08:24:39Z

ccache -s

cache directory                     /home/victor/.ccache
primary config                      /home/victor/.ccache/ccache.conf
secondary config      (readonly)    /etc/ccache.conf
cache hit (direct)                   353
cache hit (preprocessed)              71
cache miss                          1213
called for link                     1136
called for preprocessing             889
compile failed                        50
preprocessor error                    36
no input file                        420
files in cache                       437
cache size                          64.0 MB
max cache size                     100.0 MB

If the cache is empty; it has never been filled by Travis. (it has not been restored, or was never filled)
If there is not hit, the cache was un-used, something's wrong!

130s · 2017-06-27T00:23:56Z

Thank you for clarification. Doc updated.

130s · 2017-06-29T08:29:23Z

@ipa-mdl please review the updated doc.

mathias-luedtke · 2017-06-29T11:07:51Z

Docs look good now 👍
I will add tests based on @VictorLamoine's hints.

mathias-luedtke · 2017-06-29T12:05:33Z

I have modified two existing tests to check that ccache is not available by default and the cache got filled if it was enabled explicitly.
Please note that I haven't enable the Travis caching, because the tests will not benefit from it for now.

130s · 2017-06-29T17:49:06Z

.travis.yml

-    - ROS_DISTRO=indigo UPSTREAM_WORKSPACE=file  # Using default file name for ROSINSTALL_FILENAME
+    - ROS_DISTRO=indigo UPSTREAM_WORKSPACE=debian AFTER_SCRIPT='ccache 2> /dev/null && exit 1; [ "$?" = "127" ]'
+     # Using default file name for ROSINSTALL_FILENAME, test CCACHE, verify cache was filled
+    - ROS_DISTRO=indigo UPSTREAM_WORKSPACE=file CCACHE_DIR=$HOME/.ccache AFTER_SCRIPT='num=($(ccache -s | grep "files in cache")) && (( num[-1] > 0 ))'


I'm trying to understand what the commands in AFTER_SCRIPT are doing.
Locally I get this, which seems not good..:

$ num=($(ccache -s | grep "files in cache")) && (( num[-1] > 0 )) $ echo $num files

Locally I get this, which seems not good..:

actually, that's correct behaviour ;)

$(ccache -s | grep "files in cache") -> select line from ccache stats
num=($(ccache -s | grep "files in cache")) -> define an array with the data (separated by whitespace), e.g. ["files", "in", "cache", 42] (python-syntax)
num[-1] -> selects the last entry of this array
(( num[-1] > 0 )) evaluates an arithmetic expression, the exit code is used as the actual test result

echo $num just prints the first item of the array. To print all you have to use echo "${num[@]}".
echo $? will print the exit code:

$ num=($(ccache -s | grep "files in cache")) && (( num[-1] > 0 )) $ echo $? 0 # okay $ num=($(CCACHE_DIR=/tmp ccache -s | grep "files in cache")) && (( num[-1] > 0 )) $ echo $? 1 # error

My very bad...man (ba)sh is/will never be my friend...
Sounds good.

130s · 2017-06-29T22:51:49Z

So...do we want to test this feature in our own build (i.e. do we want to enable this feature .travis.yml on industrial_ci) or not?

I did not plan to add it in this PR, but perhaps in #169
But we could give it a try to see what happens

I think having job(s) that uses CCACHE_DIR is good enough for now even without testcases for it; we may not be validating the new feature (i.e. we're not checking if the new feature does what's supposed to) but we still are making there's no regression.

mathias-luedtke · 2017-06-30T06:24:45Z

we may not be validating the new feature (i.e. we're not checking if the new feature does what's supposed to) but we still are making there's no regression.

If the cache got filled, ccache should have been run :)
We don't have to test ccache itself.

mathias-luedtke mentioned this pull request Jun 18, 2017

add ccache #153

Closed

simplify ccache integration

f6629d4

mathias-luedtke force-pushed the ccache branch from 541a9b0 to f6629d4 Compare June 19, 2017 09:11

mathias-luedtke requested a review from 130s June 19, 2017 12:39

130s approved these changes Jun 23, 2017

View reviewed changes

mathias-luedtke commented Jun 24, 2017

View reviewed changes

[doc] More about ccache feature.

6c32852

130s force-pushed the ccache branch from ef69c1a to 6c32852 Compare June 27, 2017 00:23

added tests for ccache

1d1abdc

130s reviewed Jun 29, 2017

View reviewed changes

130s merged commit a102b36 into ros-industrial:master Jun 29, 2017

130s mentioned this pull request Jun 29, 2017

Cache compiled code with ccache #181

Closed

mathias-luedtke deleted the ccache branch July 2, 2017 09:25

Levi-Armstrong mentioned this pull request Mar 30, 2020

Possible issue using ccache with Clang-Tidy? #491

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simplify ccache integration #182

simplify ccache integration #182

mathias-luedtke commented Jun 18, 2017

mathias-luedtke commented Jun 18, 2017 •

edited

Loading

130s left a comment

130s Jun 23, 2017

mathias-luedtke Jun 23, 2017

mathias-luedtke commented Jun 23, 2017

130s commented Jun 23, 2017

mathias-luedtke commented Jun 23, 2017

mathias-luedtke Jun 24, 2017

mathias-luedtke Jun 24, 2017

mathias-luedtke commented Jun 25, 2017

VictorLamoine commented Jun 26, 2017 •

edited

Loading

130s commented Jun 27, 2017

130s commented Jun 29, 2017

mathias-luedtke commented Jun 29, 2017

mathias-luedtke commented Jun 29, 2017

130s Jun 29, 2017

mathias-luedtke Jun 29, 2017 •

edited

Loading

130s Jun 29, 2017

130s commented Jun 29, 2017

mathias-luedtke commented Jun 30, 2017

simplify ccache integration #182

simplify ccache integration #182

Conversation

mathias-luedtke commented Jun 18, 2017

mathias-luedtke commented Jun 18, 2017 • edited Loading

130s left a comment

Choose a reason for hiding this comment

130s Jun 23, 2017

Choose a reason for hiding this comment

mathias-luedtke Jun 23, 2017

Choose a reason for hiding this comment

mathias-luedtke commented Jun 23, 2017

130s commented Jun 23, 2017

mathias-luedtke commented Jun 23, 2017

mathias-luedtke Jun 24, 2017

Choose a reason for hiding this comment

mathias-luedtke Jun 24, 2017

Choose a reason for hiding this comment

mathias-luedtke commented Jun 25, 2017

VictorLamoine commented Jun 26, 2017 • edited Loading

130s commented Jun 27, 2017

130s commented Jun 29, 2017

mathias-luedtke commented Jun 29, 2017

mathias-luedtke commented Jun 29, 2017

130s Jun 29, 2017

Choose a reason for hiding this comment

mathias-luedtke Jun 29, 2017 • edited Loading

Choose a reason for hiding this comment

130s Jun 29, 2017

Choose a reason for hiding this comment

130s commented Jun 29, 2017

mathias-luedtke commented Jun 30, 2017

mathias-luedtke commented Jun 18, 2017 •

edited

Loading

VictorLamoine commented Jun 26, 2017 •

edited

Loading

mathias-luedtke Jun 29, 2017 •

edited

Loading