Page MenuHomePhabricator

Added benchmark suite to cmake
ClosedPublic

Authored by jasonbcox on Oct 5 2018, 19:43.

Details

Summary

Add all of the benchmark tests except checkblock. There's some additional work to fix transforming checblock's raw test data to a source file: T423

Completes T420 (except for checkblock, obviously).

Test Plan

cmake -GNinja && ninja && bitcoin-bench

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cmake
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3603
Build 5281: Bitcoin ABC Buildbot (legacy)
Build 5280: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Oct 5 2018, 22:01
schancel requested changes to this revision.Oct 5 2018, 22:03
schancel added inline comments.
src/CMakeLists.txt
18 ↗(On Diff #5295)

This really shouldn't be a cmake option. It should just be a non-default target that gets built when you run bench target.

This revision now requires changes to proceed.Oct 5 2018, 22:03

Switched from build option to target

deadalnix requested changes to this revision.Oct 14 2018, 22:48
deadalnix added inline comments.
src/bench/CMakeLists.txt
22 ↗(On Diff #5405)

You may want to EXCLUDE_FROM_ALL .

26 ↗(On Diff #5405)

You should name this bench-bitcoin . Then we can have bench-all that also run the bench for secp256k1, leveldb and whatever else.

The CMakeLists.txt from secp256k1 builds a bunch of benchmarks and can be used as an example for this, even though it is not perfect because the bench-secp256k1 just builds all the benchmarks but do not run them.

30 ↗(On Diff #5405)

You'd rather depend on the target than the file.

This revision now requires changes to proceed.Oct 14 2018, 22:48
jasonbcox marked 3 inline comments as done.

Fix according to feedback and fix build not executing via ninja

This revision is now accepted and ready to land.Oct 23 2018, 19:09
This revision was automatically updated to reflect the committed changes.