Enable ccache for speeding up build in CI #1892

SergioRAgostinho · 2017-06-22T15:02:47Z

This PR enables CCache to speed up our builds. Once a build completes, the speed up is very significant, and it should ease our time-out issues.

Note: I started working on this in #1888, but decided to submit this part separately because it's an important feature to have already enabled.

jspricke · 2017-06-22T15:07:32Z

CMakeLists.txt

@@ -182,7 +195,7 @@ if(CMAKE_COMPILER_IS_CLANG)
    SET(CMAKE_C_FLAGS "-Qunused-arguments")
  endif()
  if("${CMAKE_CXX_FLAGS}" STREQUAL "")
-    SET(CMAKE_CXX_FLAGS "-ftemplate-depth=1024 -Qunused-arguments -Wno-invalid-offsetof ${SSE_FLAGS}") # Unfortunately older Clang versions do not have this: -Wno-unnamed-type-template-args
+    SET(CMAKE_CXX_FLAGS "-Wall -Wextra -ftemplate-depth=1024 -Qunused-arguments -Wno-invalid-offsetof ${SSE_FLAGS}") # Unfortunately older Clang versions do not have this: -Wno-unnamed-type-template-args


I don't think we should set these as default (I rather vote for removing most of them).

-Qunused-argument is mandatory, otherwise we have a massive warning dump on the console.

Everything else was to be consistent with the flags defined for gcc.

But is it needed here, or could we move it to travis?

I can test without all of them and let you know what happens. I'm still unsure on what exactly you want to achieve: is it so set as less flags as possible? What about the SSE ones?

So on my local machine, with only the SSE flags active, the compiler output seems to be pretty clean so far.

The -Qunused-argument is necessary only because of a bug in ccache's interaction with clang (which eventually was fixed, but not on the version included in precise). So we can set it conditionally on the value of the TRAVIS env (which is set to true on travis builds).

Side note: what about switching to Trusty image? Maybe the bug is fixed there.

With the goal of having our tests running under a more recent platform, fully agree.

jspricke · 2017-06-22T15:24:55Z

CMakeLists.txt

@@ -79,6 +79,19 @@ SET(CMAKE_BUILD_TYPE "${CMAKE_BUILD_TYPE}" CACHE STRING
    "Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel Maintainer."
    FORCE)

+# Explicit request to use CCACHE
+if($ENV{USE_CCACHE})


Same here, I don't think forcing people to use ccache is nice. If someone wants to use it, we have a tutorial on the homepage.

This is only being triggered if the env is explicitly set. Nevertheless this is the most common option I see around. But it will required activating sudo permissions. I'm not really sure the consequences of that.

After having a look at both

https://docs.travis-ci.com/user/migrating-from-legacy/
https://docs.travis-ci.com/user/ci-environment/#Virtualization-environments

it seems like we can actually benefit from the memory increase from using sudo: required and increase the number of workers in all make tasks

SergioRAgostinho · 2017-06-23T00:55:08Z

I think we hit this problem in when moving to Travis standard infrastructure based on KVM
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56068 I think we started

I disabled SSE flags basically

SergioRAgostinho · 2017-06-23T13:48:26Z

This PR is finished in terms of the things I wanted to do. I'll just wait for the final feedback from @jspricke regarding the compiler flags, but we can always address that in a new PR later on.

jspricke · 2017-06-23T21:47:33Z

I'm fine wie the changes, but should we disable the build-apps task to make get the CI green?

SergioRAgostinho · 2017-06-24T18:03:30Z

We should. It just means it needs more work to fraction the job. I'll disable it for now and address job fractioning in a new PR.

Regarding the compiler flags comment you made, I still want to know what were your ideas (in more detail) for those.

jspricke · 2017-06-24T21:06:18Z

Coming to think of it, Adding those options is actually fine. I would propose to move it into a separate patch, though. Could you do that?

SergioRAgostinho · 2017-06-25T02:28:08Z

Yeah sure. Is it just for me to add the "-Wall -Wextra" to clang or are we revising things more deeply? If so, just open an issue to set up the discussion and I'll create the PR after.

On another note, I adopted this job stage approach because I was trying to set up a cache from the "Build Core" stage. After reading a little bit more on how the cache key is generated here, I realised that the cache was not persisting because we set envs with the task being performed (which modifies the cache key) so that we have an actual clue of what is being executed (since ENVs show up in travis GUI). If we drop this for the time being, and simply print something on the console indicating what the job does, maybe this will allow to reenable building the all the apps in acceptable time.

taketwo · 2017-06-25T09:15:47Z

If we drop this for the time being, and simply print something on the console indicating what the job does, maybe this will allow to reenable building the all the apps in acceptable time.

👍 There is a feature request for job "labels", which the Travis team has added to their roadmap. So for the time being maybe drop env variables and add more build stages (separate extended builds and tests, docs). And adopt the label feature later this year when it is implemented.

Another Travis option we may add to save a tiny bit of time: clone depth.

SergioRAgostinho · 2017-06-25T14:40:40Z

I made some tests yesterday in another branch and the caching feature from ccache is not really storing that much yet. Better to just merge this one in the meantime.

Another Travis option we may add to save a tiny bit of time: clone depth.

Isn't this gonna cut the build very residually?

taketwo · 2017-06-25T14:50:15Z

Isn't this gonna cut the build very residually?

Yes, will only save several seconds and some bandwidth for Travis. But why not.

The caching feature from ccache is not really storing that much yet.

So in the end of the day we are not gaining anything here?

SergioRAgostinho · 2017-06-25T14:52:14Z

So in the end of the day we are not gaining anything here?

I can't tell at the moment but something is weird. There's no single ccache hit... You can check what I'm doing here https://travis-ci.org/SergioRAgostinho/pcl

SergioRAgostinho · 2017-06-25T14:57:14Z

Basically there's no single ccache hit whenever we use clang on linux and I don't think ccache is working properly on osx, just with instructions specified in Travis.

taketwo · 2017-06-25T15:05:03Z

I'm afraid I'm not much of a help here, never used ccache, even locally.

SergioRAgostinho · 2017-06-25T22:58:47Z

It's kinda weird. I tried it locally on my environment and thing works exactly as described on Travis' instructions.

On osx, ccache is simply not being picked up properly by cmake and the cache is not written at all. I even tried to explicitly set the compiler path to ccache, but somehow it still resolves to the the clang binary directly.

SergioRAgostinho · 2017-06-25T23:40:22Z

Ok, guys I'm just waiting for you to merge this one, for me to open another one, enabling a couple of jobs in osx.

SergioRAgostinho force-pushed the ccache branch from 30b4a1f to 427c251 Compare June 22, 2017 15:05

jspricke reviewed Jun 22, 2017

View reviewed changes

Enable ccache for speeding up build in CI

8af8538

SergioRAgostinho force-pushed the ccache branch 3 times, most recently from 2c993ac to f9172fa Compare June 23, 2017 13:41

Enable travis stages and disable building apps for now

92d3fd7

SergioRAgostinho force-pushed the ccache branch from 87ecff9 to 92d3fd7 Compare June 25, 2017 00:12

jspricke merged commit 08e9ebc into PointCloudLibrary:master Jun 26, 2017

SergioRAgostinho mentioned this pull request Jun 26, 2017

[WIP] Enabled OSX builds in Travis #1899

Closed

taketwo mentioned this pull request Sep 11, 2017

[CMAKE] Tiny useless change. #1991

Closed

SergioRAgostinho deleted the ccache branch September 21, 2017 01:43

SergioRAgostinho mentioned this pull request Nov 26, 2017

Modification of the Travis tasks #699

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable ccache for speeding up build in CI #1892

Enable ccache for speeding up build in CI #1892

SergioRAgostinho commented Jun 22, 2017

jspricke Jun 22, 2017

SergioRAgostinho Jun 22, 2017

jspricke Jun 22, 2017

SergioRAgostinho Jun 22, 2017

SergioRAgostinho Jun 22, 2017

taketwo Jun 23, 2017 •

edited

SergioRAgostinho Jun 23, 2017 •

edited

jspricke Jun 22, 2017

SergioRAgostinho Jun 22, 2017

SergioRAgostinho Jun 22, 2017

SergioRAgostinho commented Jun 23, 2017

SergioRAgostinho commented Jun 23, 2017

jspricke commented Jun 23, 2017

SergioRAgostinho commented Jun 24, 2017

jspricke commented Jun 24, 2017 via email

SergioRAgostinho commented Jun 25, 2017 •

edited

taketwo commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017

taketwo commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017

taketwo commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017 •

edited

SergioRAgostinho commented Jun 25, 2017

Enable ccache for speeding up build in CI #1892

Enable ccache for speeding up build in CI #1892

Conversation

SergioRAgostinho commented Jun 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taketwo Jun 23, 2017 • edited

Choose a reason for hiding this comment

SergioRAgostinho Jun 23, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergioRAgostinho commented Jun 23, 2017

SergioRAgostinho commented Jun 23, 2017

jspricke commented Jun 23, 2017

SergioRAgostinho commented Jun 24, 2017

jspricke commented Jun 24, 2017 via email

SergioRAgostinho commented Jun 25, 2017 • edited

taketwo commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017

taketwo commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017

taketwo commented Jun 25, 2017

SergioRAgostinho commented Jun 25, 2017 • edited

SergioRAgostinho commented Jun 25, 2017

taketwo Jun 23, 2017 •

edited

SergioRAgostinho Jun 23, 2017 •

edited

SergioRAgostinho commented Jun 25, 2017 •

edited

SergioRAgostinho commented Jun 25, 2017 •

edited