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/travis timeout #45

Merged
merged 8 commits into from
Jun 3, 2019
Merged

Fix/travis timeout #45

merged 8 commits into from
Jun 3, 2019

Conversation

dellaert
Copy link
Member

@dellaert dellaert commented Jun 2, 2019

I refactored slightly, to use more direct configuration of cmake.
I also disabled unstable for gcc tests build, it comes in right under 50 mins ....

One possible speedup is using the gold linker, see http://www.bitsnbites.eu/faster-c-builds/

@dellaert dellaert added the enhancement Improvement to GTSAM label Jun 2, 2019
@dellaert dellaert requested a review from jlblancoc June 2, 2019 15:06
@dellaert dellaert self-assigned this Jun 2, 2019
@ghost
Copy link

ghost commented Jun 2, 2019

Btw, is ccache actually working? Looking at the last few builds on develop of gcc + examples, they all took 24+ minutes, which is not what I would expect. For example, the build after you merged PR #40 should have gone much faster because it didn't have to rebuild base, geometry, etc.

@dellaert
Copy link
Member Author

dellaert commented Jun 2, 2019

I don’t think we enabled ccache yet, is described here. I can add it to this PR. Otherwise ok?

@dellaert
Copy link
Member Author

dellaert commented Jun 2, 2019

@dellaert
Copy link
Member Author

dellaert commented Jun 2, 2019

By the way, caching will only help for small changes, right? I think we still have to make sure that a full rebuild fits within 50 minutes.

@dellaert
Copy link
Member Author

dellaert commented Jun 2, 2019

OK,

  • I saw ccache was enable by Jose, nice
  • I added Mac OS install
  • it seems ccache also needs "Build pushed branches" ON, which I did.

@chrisbeall
Copy link
Member

Hmm, I thought it was enabled because we have:

gtsam/.travis.yml

Lines 1 to 2 in 823c8bd

language: cpp
cache: ccache

That's exactly what https://docs.travis-ci.com/user/caching says to do. Our cmake config does pick it up, and says it will use it.
Correct, it'll only help with small changes. As long as you're not touching base and geometry, speedup is huge.

@chrisbeall
Copy link
Member

Just fyi: As far as gtsam is concerned, ccache is turned on by default if we detect it (unless we're on Windows or generate an Xcode project):

gtsam/CMakeLists.txt

Lines 71 to 73 in 823c8bd

if(NOT MSVC AND NOT XCODE_VERSION)
option(GTSAM_BUILD_WITH_CCACHE "Use ccache compiler cache" ON)
endif()

@chrisbeall
Copy link
Member

Ah, there's an easy way to check whether cache was used. At the end of shell script, run ccache -s to print statistics.

@dellaert
Copy link
Member Author

dellaert commented Jun 2, 2019

It seems to be used after I enabled branches. First two builds were 3/4 mins only!
I'll add ccache -s and remove the forcing flag, making sure to check whether it actually works.

@dellaert
Copy link
Member Author

dellaert commented Jun 2, 2019

PS, brew update takes almost 5 mins all by itself. Are we sure it's needed, @jlblancoc ?

@jlblancoc
Copy link
Member

jlblancoc commented Jun 2, 2019 via email

@chrisbeall
Copy link
Member

Looks like it definitely worked on Linux, but mac is still slow.
If I understand the documentation correctly, I don't think we run the risk of poisoning the cache between configurations, because we have unique OS, compiler, and environment variables in each build.

@chrisbeall
Copy link
Member

But we get two sets of builds now; that's not good :-(
image

Hoping that the latter will fix cache issue on Mac. And who uses gcc on Mac anyway?
@jlblancoc
Copy link
Member

jlblancoc commented Jun 3, 2019 via email

@dellaert
Copy link
Member Author

dellaert commented Jun 3, 2019

Finally the clang one passed on Mac and wrote the cache. This story is not yet over I think, but I will merge for now.

@dellaert dellaert merged commit 228ac65 into develop Jun 3, 2019
@dellaert dellaert deleted the fix/travis_timeout branch June 5, 2019 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to GTSAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants