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

Travis - Faster CI checks #464

Merged

Conversation

mnaamani
Copy link
Member

@mnaamani mnaamani commented May 23, 2020

This will reduce total build times (as well as storage usage in ./target/ making caching more effective) because cargo tests, clippy and main build will all be in release mode (and therefore re-use the already built packages)

Reduced total build time from 1h45min on average to 45min, but more importantly reduced back to a single job so we can have more concurrent job across open PRs, reducing wait time for any one PR's check to complete.

In the process of working on this PR, rust nightly-2020-05-26 toolchain came out which broke the build so had to lock to older version of the toolchain, nightly-2020-050-23

this will reduce total build times in travis, and when running git hooks
@mnaamani
Copy link
Member Author

mnaamani commented May 24, 2020

Was able to shave off ~30min from build time.
Now that the target folder is smaller, perhaps enabling cargo caching will work better.
Will push another commit to test.

@mnaamani
Copy link
Member Author

Doing some tests on caching, so pushed a commit with changes that don't touch any rust code to see what the build time is like...

@mnaamani
Copy link
Member Author

mnaamani commented May 25, 2020

Enabling or disabling the cargo cache isn't really having much of an effect, despite the logs clearly showing the cache being setup and updated at the start and end of a job.

Compiling of the whole runtime still goes on even if no changes to any file in the cargo workspace occurs.

One thing I did notice however is the name of the tarball that represents the cache seems to be identical for all the jobs with the same os = linux. Could that be the reason.. will continue experiment.

@mnaamani mnaamani marked this pull request as draft May 25, 2020 07:56
@mnaamani mnaamani removed the request for review from shamil-gadelshin May 25, 2020 07:57
@mnaamani
Copy link
Member Author

After some extensive testing with various approaches, decided to fallback on simply running tests, clippy and build using nightly and using same release build type, and enabling cargo cache. This is giving the best build time overall.

I also decided that because of subtle issue with how WASM blob is compiled, it is safer to start with a clean build (no cargo caching) when preparing release builds and will move that task to a separate system/platform.

This will also reduce number of jobs concurrent when a PR is opened, and given we have much more activity now in the monorepo, it will shorten the time for a CI job to actually start and complete. So no need to wait around for CI checks to complete before merging..

@mnaamani
Copy link
Member Author

I also decided that because of subtle issue with how WASM blob is compiled, it is safer to start with a clean build (no cargo caching) when preparing release builds and will move that task to a separate system/platform.

This fix might make the issue go away at least when building locally.

paritytech/substrate@51ead38#diff-90e1d607b4e47cba828a5e70b658b7a1

@mnaamani
Copy link
Member Author

Struggled a bit last 2 days with constant failures. After going down the wrong path looking for dependencies that might have broken the build, it turned out to be due to release of latest nightly.

Resolved by locking to last best known working version. Also opened an issue upstream paritytech/substrate#6167

Learned a lot working on this PR, I'm thinking to keep the commit history for future reference, instead of squashing.

@mnaamani mnaamani marked this pull request as ready for review May 27, 2020 21:45
This way env var BUILD_DUMMY_WASM_BINARY with cargo build --release
@mnaamani mnaamani changed the title Run cargo tests and clippy in release mode Travis - Faster CI checks May 28, 2020
Copy link
Contributor

@shamil-gadelshin shamil-gadelshin left a comment

Choose a reason for hiding this comment

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

LGTM

@shamil-gadelshin shamil-gadelshin merged commit 7306a7e into Joystream:development May 28, 2020
@mnaamani mnaamani deleted the speedup-travis-and-hooks branch May 28, 2020 08:40
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.

2 participants