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

Enable Travis cache #1819

Merged
merged 2 commits into from Aug 15, 2017
Merged

Enable Travis cache #1819

merged 2 commits into from Aug 15, 2017

Conversation

kkm000
Copy link
Contributor

@kkm000 kkm000 commented Aug 14, 2017

Using ccache to cache object files. Also, the work is split into two jobs: one compiles everything (but skips linking binaries), and another compiles and runs tests only in libraries.

I am getting as low as 5 minutes in the first job without changes in source files, around 25 in the second. But Travis is quite inconsistent in performance: One run took almost 40 minutes, although the cache hit ratio was nominal.

Most of the changes are related to quoting the variable CXX properly.

Notable omissions:

  • Compiling in single precision only. Old way was randomize on each run, but that would rub ccache's fur the wrong way. Do we need both? I'll add separate jobs for the other mode if need both.
  • Not running the target ext_test. It adds a lot to compile time, partly because portaudio, but runs only 1 test in src/online2.

Closes #1805

@@ -87,7 +94,7 @@ sclite: sclite_compiled
.PHONY: sclite_compiled
sclite_compiled: sctk sctk_configured
cd sctk; \
$(MAKE) CC=$(CC) CXX=$(CXX) all && $(MAKE) install && $(MAKE) doc
$(MAKE) CC="$(CC)" CXX="$(CXX)" all && $(MAKE) install && $(MAKE) doc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do CC and CXX need to be quoted here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, because they can be (and CXX is) a multi-token value. ccache wraps a compiler:

- CXX="ccache clang++-3.8 -Qunused-arguments -fcolor-diagnostics -Wno-tautological-compare"

@danpovey
Copy link
Contributor

I don't object to anything specific in it, but @jtrmal may want to have a look; and it's still timing out.

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 14, 2017

Yes, I just restarted it for the 3rd time. I did see it succeed though, and when caches warms up, timeouts will be less likely. But this is not a complete solution; tests should be able to finish with a cold cache.

I think the next step is to split testing between two or more jobs. I need to tally a successful run to understand what libs have the least interdependencies, and which of them have the longest running tests.

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 14, 2017

Huh. Travis is mind-boggling. The testing job timed out thrice, then on the fourth run completed in 15 minutes (and that's with a completely cold cache!). Test run times would be more those of a high-end CPU. So much variance that its hard to even plan splitting the test job into multiple.

@jtrmal
Copy link
Contributor

jtrmal commented Aug 14, 2017 via email

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 14, 2017

If that continues to be a problem, and alternative is e. g. Jenkins on Amazon EC. But that requires some budget and, more importanly, some work, and I can think of the way to spend time better... Let's see if Travis is still an option with all its quirks and vagaries.

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 15, 2017

@jtrmal, PTAL if you have time. I am not pushing you, just pointing this is a complete change ready for a code review, and I think this is already better that what we already have, and will in any case improve the chances of a CI job running to completion. Travis pulls the master branch cache for pull requests when they do not have one on their own, then reuses this PR-specific cache if the PR branch is changed, so having a warm cache for the master branch will help.

@jtrmal
Copy link
Contributor

jtrmal commented Aug 15, 2017 via email

@jtrmal
Copy link
Contributor

jtrmal commented Aug 15, 2017

@danpovey LGTM, please merge.

@kkm000
Copy link
Contributor Author

kkm000 commented Aug 15, 2017

Thanks!

@danpovey
Copy link
Contributor

Thanks! Merging.

@danpovey danpovey merged commit eaebe08 into kaldi-asr:master Aug 15, 2017
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants