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

travis: use ccache to speed up build #3649

Merged
merged 3 commits into from Apr 15, 2018

Conversation

Projects
None yet
7 participants
@liushuyu
Contributor

liushuyu commented Apr 11, 2018

In this PR, ccache is used to speed up future builds.

The first commit of this PR and when merged into main repository however, will take around 2x time to set up/generate/compile (whatever you want to call this process) the cache for the first time.

Some evidence for showing how much the caching system can speed up? https://travis-ci.org/liushuyu/citra/builds/365178556

Further thoughts:
Before, most of the time were spent on building citra, now it'll be updating and installing dependencies. Since we are using Docker images, we can create a Docker image that contains our dependencies and modifications then just remember to update the image often.


This change is Reviewable

@cluezbot

This comment has been minimized.

cluezbot commented Apr 11, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-11T15:44:47Z: 8b858f1...liushuyu:38c1c2848380e9623e9c1079f354263bcfcc45c0

@liushuyu

This comment has been minimized.

Contributor

liushuyu commented Apr 12, 2018

I noticed Appveyor only allow 1 concurrent job at a time and I am thinking to move MinGW build to Travis CI platform to reduce the build time, there are currently several issues during my testing:

  • Currently bundled libressl is outdated, latest version is 2.7.2 and this release corrected several issues regarding build system (CMakeLists).
  • Cross-compiling would require some workarounds to make the project builds
  • Packaging and running tests are no easy task(s).

After addressing above problems, the build time should reduce at least 30%

@jroweboy

This comment has been minimized.

Member

jroweboy commented Apr 12, 2018

@liushuyu yusss. so happy someone is looking into our CI situation :)

Just to let you know, we've had a few internal discussions about moving to a self hosted buildbot instance after we had issues with both appveyor and travis. The idea is to keep at least appveyor still around for testing msvc, but to run our own infrastructure so we have more control over the build queue. One idea was to use mingw and osxcross in docker so that we can use a linux vm to build for all release platforms (the entire web infrastructure runs on a few linode linux vms right now and we can run the docker builds on there), and the other idea was to just self host a win and osx vm somewhere (while still using linode for linux). Theres positives and negatives to both as you are probably aware, but if the cross compilation + ccache works, I say full steam ahead on having our own docker and buildbot for all of this.

Anyway, I know its not related to this PR, but it seems you are at least a little interested in improving the build system, so I figured I'd let you know some of the talk around where we were thinking of going next. If you'd like to discuss this with us, feel free to respond here on github, or hit us up on IRC #citra-dev on freenode or on the discord. ❤️

@liushuyu

This comment has been minimized.

Contributor

liushuyu commented Apr 12, 2018

One idea was to use mingw and osxcross in docker so that we can use a linux vm to build for all release platforms

Actually I can agree on the idea using MinGW to cross compile Windows (R) binaries. But cross compile for macOS? No, it was really a bad experience, that's because how Apple dealing with things: you have to purchase their device running their OS to build apps for them.

or hit us up on IRC #citra-dev on freenode or on the discord.

Joined Discord channel

@j-selby

LGTM, pending minor tweaks

@@ -1,4 +1,4 @@
#!/bin/bash -ex
mkdir "$HOME/.ccache" || true

This comment has been minimized.

@j-selby

j-selby Apr 13, 2018

Contributor
mkdir -p "$HOME/.ccache"

? - no need to ignore directory existing errors here

This comment has been minimized.

@liushuyu

liushuyu Apr 13, 2018

Contributor

See the shebang ( #!/bin/bash -ex) the flags -ex are enabled.
If there's an error (directory already exists), the whole build will stop (the effect of set -e)

This comment has been minimized.

@j-selby

j-selby Apr 14, 2018

Contributor

No, the -p flag will not error if the directory exists

docker pull ubuntu:18.04
docker run -e ENABLE_COMPATIBILITY_REPORTING -v $(pwd):/citra ubuntu:18.04 /bin/bash -ex /citra/.travis/linux-frozen/docker.sh
docker run -e ENABLE_COMPATIBILITY_REPORTING -v $(pwd):/citra -v "$HOME/.ccache":/root/.ccache ubuntu:18.04 /bin/bash -ex /citra/.travis/linux-frozen/docker.sh

This comment has been minimized.

@j-selby

j-selby Apr 13, 2018

Contributor

Likely unintentional extra space after 18.04

@@ -1,3 +1,3 @@
#!/bin/bash -ex
docker run -e ENABLE_COMPATIBILITY_REPORTING -v $(pwd):/citra ubuntu:18.04 /bin/bash -ex /citra/.travis/linux/docker.sh
mkdir "$HOME/.ccache" || true

This comment has been minimized.

@j-selby

j-selby Apr 13, 2018

Contributor

Ditto as above

@@ -20,7 +20,7 @@ echo y | sh cmake-3.10.1-Linux-x86_64.sh --prefix=cmake
export PATH=/citra/cmake/cmake-3.10.1-Linux-x86_64/bin:$PATH
mkdir build && cd build
cmake .. -DCMAKE_BUILD_TYPE=Release -DCITRA_ENABLE_COMPATIBILITY_REPORTING=${ENABLE_COMPATIBILITY_REPORTING:-"OFF"}
CC=/usr/lib/ccache/gcc CXX=/usr/lib/ccache/g++ cmake .. -DCMAKE_BUILD_TYPE=Release -DCITRA_ENABLE_COMPATIBILITY_REPORTING=${ENABLE_COMPATIBILITY_REPORTING:-"OFF"}

This comment has been minimized.

@j-selby

j-selby Apr 13, 2018

Contributor

Using environmental variables for this seems a bit dirty - CMake flag instead?

This comment has been minimized.

@liushuyu

liushuyu Apr 13, 2018

Contributor

Despite this, do you want to add an option to the root CMakeLists.txt that uses ccache automatically when enabled?

This comment has been minimized.

@Subv

Subv Apr 14, 2018

Member

IMO this is fine, that's the usual way to invoke ccache with cmake

# Get a recent version of CMake
wget https://cmake.org/files/v3.10/cmake-3.10.1-Linux-x86_64.sh
echo y | sh cmake-3.10.1-Linux-x86_64.sh --prefix=cmake
export PATH=/citra/cmake/cmake-3.10.1-Linux-x86_64/bin:$PATH
mkdir build && cd build
cmake .. -DCMAKE_BUILD_TYPE=Release -DENABLE_QT_TRANSLATION=ON -DCITRA_ENABLE_COMPATIBILITY_REPORTING=${ENABLE_COMPATIBILITY_REPORTING:-"OFF"}
CC=/usr/lib/ccache/gcc CXX=/usr/lib/ccache/g++ cmake .. -DCMAKE_BUILD_TYPE=Release -DENABLE_QT_TRANSLATION=ON -DCITRA_ENABLE_COMPATIBILITY_REPORTING=${ENABLE_COMPATIBILITY_REPORTING:-"OFF"}

This comment has been minimized.

@j-selby

j-selby Apr 13, 2018

Contributor

Ditto as above

@liushuyu liushuyu force-pushed the liushuyu:ci-cache branch from 38c1c28 to d679469 Apr 14, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Apr 14, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-14T01:10:32Z: 7f408ee...liushuyu:d6794694c98b68d03e9e799a548ab9c290d1823d

@liushuyu liushuyu force-pushed the liushuyu:ci-cache branch from d679469 to 2193e9b Apr 14, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Apr 14, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-14T01:10:48Z: 7f408ee...liushuyu:2193e9b742d6e74206cf5c7e33862c8e69d96649

@cluezbot

This comment has been minimized.

cluezbot commented Apr 14, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-14T02:30:47Z: 7f408ee...liushuyu:0d3983bc66c658b1d2a735e87056d5d43d7391e3

@liushuyu

This comment has been minimized.

Contributor

liushuyu commented Apr 14, 2018

Okay, everything @j-selby has pointed out is fixed or corrected

@j-selby

LGTM now! Thanks for working on buildbot stuff.

@liushuyu

This comment has been minimized.

Contributor

liushuyu commented Apr 15, 2018

Anyone still has any suggestions?

@wwylele wwylele merged commit 17e14d5 into citra-emu:master Apr 15, 2018

2 of 3 checks passed

code-review/reviewable 7 files, 5 discussions left (j-selby, liushuyu, Subv)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment