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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Oct 5 2018, 19:43
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 5 2018, 19:43
jasonbcox edited the summary of this revision. (Show Details)Oct 5 2018, 19:44
schancel accepted this revision.Oct 5 2018, 22:01
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
jasonbcox updated this revision to Diff 5405.Oct 12 2018, 21:32

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 updated this revision to Diff 5421.Oct 15 2018, 20:46
jasonbcox marked 3 inline comments as done.

Fix according to feedback and fix build not executing via ninja

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