This repository has been archived by the owner on May 9, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
de46fcc
to
86e5703
Compare
ebded9c
to
f63e059
Compare
acrrd
reviewed
Mar 18, 2021
bbbe556
to
3514dd4
Compare
your pr summary is awesome 👍 we should put a link to this readme/pr directly in the ci yaml file. |
Thanks 😊. I can do that! |
ed08f13
to
85d6a2a
Compare
janpetschexain
approved these changes
Apr 7, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👍
acrrd
approved these changes
Apr 7, 2021
Co-authored-by: janpetschexain <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal:
We want to test our android/ios libraries on different cpu architectures
in order to identify errors early in development.
What we have achieved:
We can compile and test our android libraries on different cpu architectures by using an emulator.
However, we can only compile the iOS libraries because there is no emulator for it.
We could run our tests on the iOS simulator, but it uses an architecture (x86_64)
which is not used in any iOS device. So it is not worth it.
Implementation details
Use a build matrix for testing/building android/ios libraries on different cpu architectures
There are several advantages to using a build matrix instead of one job that builds/tests all of the libraries one at a time:
Optimised android test
We noticed that the execution time of the tests on the emulator is significantly reduced
if we compile our tests in
release
mode. However, in release mode thedebug assertions
are removedwhich means that we lose some checks. So the idea was to compile it in debug mode (to keep the
debug assertions
) and only setopt-level
to3
(to turn on all optimisations).However, this turned out to be more difficult than expected. In the end we decided to use
RUSTFLAGS="-C opt-level=3 -C debug-assertions=yes"
. More details at the end of the description.test-android-libs without caching / debug mode
17m 25s
59m 5s
test-android-libs without caching / release mode
11m 32s
37m 44s
test-android-libs with caching / release mode
4m 38s
13m 7s
We have activated the option
-- -Z unstable-options --report-time
so that if a test suddenly becomes slow again, we can recognize it immediately.Using
cross
for testingcross
makes it easy to test our code on different cpu architectures. Under the hood it runs a docker container that runs an emulator (qemu).Features/Limitations of
cross
cross
mounts the repo into the container, you cannot execute it in a folder belonging to a member who depends on another memberFor example:
does not work. It cannot find the directory of
rubert
, since it is not mounted into the container. So you have to runUsing
cargo-ndk
for building the release binariesTo build the final android libraries we will keep using
cargo-ndk
because it has some advantages overcross
:ndk
we can set the ndk api level (incross
it is hardcoded in the dockerfile)ndk
automaticallystrip
s the binary (with the right strip binary) + moves it into thejniLib
folder structureWe will do the integration of
cargo-ndk
in a separate pr.Why we chose
RUSTFLAGS
instead of[profile.test]
TL;DR:
The test profile only applies to our library but not on the dependencies.
cargo book
cargo test
Dependencies
If
opt-level
is missing the compiler will by default use-C opt-level=0
which implies
-C debug-assertions=on
Our Library
default:
-C opt-level=0
->-C debug-assertions=on
cargo test
&Dependencies
-C opt-level=3
impliesdebug-assertions=off
but cargo correctly uses the default value ofdebug-assertions=on
inprofile.dev
.Our Library
cargo test
&Dependencies
Our Library
Cool, we've reached our goal, haven't we? Not really.
If we add
opt-level=3
toprofile.dev
, the complication times during localdevelopment will be longer.
An alternative would be to use
RUSTFLAGS
. Let's take a look.RUSTFLAGS="-C opt-level=3" cargo test
Dependencies
Our Library
Hmmm cargo does not apply the default values of the profiles anymore.
What happens if we and
profile.dev
again?RUSTFLAGS="-C opt-level=3" cargo test
&Dependencies
debug-assertions=on
is back and we haveopt-level=3
twice.Which of the two
opt-level=3
will it actually choose?Our Library
Let's set
opt-level
to3
in theCargo.toml
and call cargo withRUSTFLAGS="-C debug-assertions=no"
.RUSTFLAGS="-C debug-assertions=no" cargo test
&Dependencies
The test did not fail, so the
debug-assertions
are turned off which means that the lastdebug-assertions
flag overwrites the previous flag.In order to have both
opt-level=3
anddebug-assertions=yes
we need to runRUSTFLAGS="-C opt-level=3 -C debug-assertions=yes" cargo test
Dependencies
Our Library