CI/build: enable coverage for bin_tps_evaluation
Context
Instrumenting consistently
The build jobs should preferable make all calls to dune build
using the same value for --instrument-with
: if not, dune will have to rebuild common targets with different preprocessing. Currently, in the CI build job, we have:
1. `dune build @runtest_dune_template`
2. `make all build-sandbox`
3. `dune build src/bin_tps_evaluation`
The call to make
will respect the environment variable COVERAGE_OPTIONS
. In x86_64, this variable is set to enable coverage instrumentation. The other two calls to dune do not pass this option. Consequently, the calls to dune provoked by make
in the second point does not reuse common targets from runtest_dune_template
. Similarly, the call to dune
in step 3. Does not reuse common targets from the step 2.
Using the and_recount.sh
script, we can see the number of files that are recreated in the _build
folder depending on whether we add instrumentation to step 1 and 3:
- Original:
| step | instrumentation | new/updated files |
|-----------------------+-----------------+-------------------|
| runtest_dune_template | no | 5159 |
| make all build-test | yes | 28362 |
| bin_tps_evaluation | no | 4668 |
|-----------------------+-----------------+-------------------|
| | | 38189 |
- Add instrumentation to the third step
bin_tps_evaluation
| step | instrumentation | new/updated files |
|-----------------------+-----------------+-------------------|
| runtest_dune_template | no | 5159 |
| make all build-test | yes | 28362 |
| bin_tps_evaluation | yes | 937 |
|-----------------------+-----------------+-------------------|
| | | 34458 |
- Add instrumentation to the first and third step:
| step | instrumentation | new/updated files |
|-----------------------+-----------------+-------------------|
| runtest_dune_template | yes | 7944 |
| make all build-test | yes | 24274 |
| bin_tps_evaluation | yes | 936 |
|-----------------------+-----------------+-------------------|
| | | 33154 |
Not building tests
In addition, I don't understand why the build
jobs run make build-test
. No build artifacts from this target will be reused in the
subsequent pipeline stages (because only the build
jobs use the
cache).
Instead, the integration tests (pytest / tezt) uses the octez-
binaries stored as a gitlab artifact and restored in the test stages.
Flextesa tests have their own binary, which can be created with make test-sandboxes
.
The unit tests rebuild their own tests binaries, and do not re-use any cache or artifacts from the build stage.
So another optimization is to not run make build-test
: the unit
tests will ensure that the tests are buildable anyway.
So as a final optimization, I replace build-test
by
build-sandbox
that creates the binaries for flextesa.
| step | instrumentation | new/updated files |
|-----------------------+-----------------+-------------------|
| runtest_dune_template | yes | 7969 |
| make all build-sand.. | yes | 22919 |
| bin_tps_evaluation | yes | 919 |
|-----------------------+-----------------+-------------------|
| | | 31807 |
Is it any faster?
I've run test pipelines:
- notpscov: only build job from master
-
tpscov: only build job, instrument
bin_tps_evaluation
-
tpscov2: only build job, instrument
bin_tps_evaluation
andruntest_dune_template
, and do not makebuild-test
.
In addition to these ones, to benchmark the cache, I've run
that are the same as the jobs above but with the cache disabled.
|---------------------+----------------------------------|
| Job | Duration (mean) |
|---------------------+----------------------------------|
| notpscov (n=11) | 19m59.85s |
| tpscov (n=11) | 14m43.77s |
| tpscov2 (n=12) | 10m44.16s |
| nc_notpscov (n=10) | 24m53.42s |
| nc_tpscov (n=10) | 22m38.59s |
| nc_tpscov2 (n=11) | 19m0.12s |
|---------------------+----------------------------------|
Note that these use the gitlab shared runners which means that durations are not directly comparable to those of the tezos/tezos CI. Nonetheless, it seems that the changes proposed in this MR makes the build faster and that the cache is useful.
Open questions
Are there any drawbacks to adding coverage instrumentation to
bin_tps_evaluation
? It makes the binary run slightly slower, but it
will probably be used to benchmark a node that is also instrumented anyway!
Manually testing the MR
Checklist
-
Document the interface of any function added or modified (see the coding guidelines) -
Document any change to the user interface, including configuration parameters (see node configuration) -
Provide automatic testing (see the testing guide). -
For new features and bug fixes, add an item in the appropriate changelog ( docs/protocols/alpha.rst
for the protocol and the environment,CHANGES.rst
at the root of the repository for everything else). -
Select suitable reviewers using the Reviewers
field below. -
Select as Assignee
the next person who should take action on that MR