Skip to content

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 and runtest_dune_template, and do not make build-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
Edited by Arvid Jakobsson

Merge request reports