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 3596
Build 5267: Bitcoin ABC Buildbot (legacy)
Build 5266: 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

You may want to EXCLUDE_FROM_ALL .

26

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

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.