bench: Suppress output when running with `-sanity-check` option #26481

hebasto · 2022-11-10T16:28:18Z

This change allows to simplify CI tests, and makes it easier to integrate the bench_bitcoin binary into CMake custom targets or commands, as COMMAND does not support output redirection.

This change allows to simplify CI tests, and makes it easier to integrate the `bench_bitcoin` binary into CMake custom targets or commands, as `COMMAND` does not support output redirection

DrahtBot · 2022-11-12T00:25:36Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	aureleoules
Concept ACK	shaavan, kevkevinpal

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

shaavan

Concept ACK

This change is nice cleanup to master. Since sanity-checking the benchmark result is useless and will be redirected to NUL, it is better to suppress them and remove the need for output redirection.

The one thing I am not sure about is this:

as COMMAND does not support output redirection.

I assume by commands, you mean add_custom_commands here. I did a little reading up on this, but this statement might not be accurate.

https://stackoverflow.com/a/31582345

But it's not true for the add_custom_command. Redirection should work there as expected.

https://stackoverflow.com/a/36305542

Unlike to commands in add_custom_command, which are executed as a part of makefile receipts (that is, in the context of some shell), tests are executed directly by CTest, without any shell involved. So, shell mechanisms don't work for tests.

So would you please provide some sources to confirm this claim in the description? Thank you.

kevkevinpal · 2022-12-23T01:59:55Z

Tested ACK

ran this: ./src/bench/bench_bitcoin -sanity-check -priority-level=high
and only got this line
Running with -sanity-check option, output is being suppressed as benchmark results will be useless.

then I ran: ./src/bench/bench_bitcoin -priority-level=high
and got the output

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|       58,488,580.00 |               17.10 |    2.3% |      0.63 | `AddrManAdd`
|      144,473,637.00 |                6.92 |    6.3% |      1.57 | :wavy_dash: `AddrManAddThenGood` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
|          249,270.00 |            4,011.71 |    9.3% |      0.01 | :wavy_dash: `AddrManGetAddr` (Unstable with ~2.7 iters. Increase `minEpochIterations` to e.g. 27)
|              157.28 |        6,358,068.29 |    9.0% |      0.01 | :wavy_dash: `AddrManSelect` (Unstable with ~6,021.4 iters. Increase `minEpochIterations` to e.g. 60214)
|          447,008.50 |            2,237.09 |    1.8% |      0.01 | `AssembleBlock`

|             ns/byte |              byte/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              111.39 |        8,977,617.86 |    1.4% |      0.01 | `Base58CheckEncode`
|               25.14 |       39,771,942.56 |    2.0% |      0.01 | `Base58Decode`
|               83.38 |       11,993,484.31 |    1.5% |      0.01 | `Base58Encode`
|               13.88 |       72,048,351.15 |    4.3% |      0.01 | `Bech32Decode`
|               32.47 |       30,800,157.58 |    2.0% |      0.01 | `Bech32Encode`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              286.08 |        3,495,567.40 |    7.4% |      0.01 | :wavy_dash: `BenchLockedPool` (Unstable with ~3,633.0 iters. Increase `minEpochIterations` to e.g. 36330)
|               35.21 |       28,402,277.93 |    1.2% |      0.01 | `BenchTimeDeprecated`
|               34.60 |       28,902,932.85 |    2.0% |      0.01 | `BenchTimeMillis`
|               35.19 |       28,416,030.97 |    1.2% |      0.01 | `BenchTimeMillisSys`
|                2.36 |      423,272,811.99 |    1.0% |      0.01 | `BenchTimeMock`
|      124,648,173.00 |                8.02 |    3.5% |      1.39 | `BlockToJsonVerbose`
|       56,973,872.00 |               17.55 |    1.4% |      0.63 | `BlockToJsonVerboseWrite`

|              ns/job |               job/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              844.28 |        1,184,435.76 |    5.2% |      0.31 | :wavy_dash: `CCheckQueueSpeedPrevectorJob` (Unstable with ~10.9 iters. Increase `minEpochIterations` to e.g. 109)

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              623.04 |        1,605,036.29 |    6.7% |      0.01 | :wavy_dash: `CCoinsCaching` (Unstable with ~1,510.2 iters. Increase `minEpochIterations` to e.g. 15102)

|             ns/byte |              byte/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                1.69 |      590,329,986.59 |    7.4% |      0.02 | :wavy_dash: `CHACHA20_1MB` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
|                2.00 |      499,795,120.75 |    3.9% |      0.01 | `CHACHA20_256BYTES`
|                2.15 |      464,722,164.36 |    0.2% |      0.01 | `CHACHA20_64BYTES`
|                6.00 |      166,640,656.61 |    1.8% |      0.07 | `CHACHA20_POLY1305_AEAD_1MB_ENCRYPT_DECRYPT`
|                2.98 |      335,652,695.54 |    0.7% |      0.03 | `CHACHA20_POLY1305_AEAD_1MB_ONLY_ENCRYPT`
|                7.89 |      126,815,704.95 |    3.1% |      0.01 | `CHACHA20_POLY1305_AEAD_256BYTES_ENCRYPT_DECRYPT`
|                3.86 |      258,855,192.23 |    1.2% |      0.01 | `CHACHA20_POLY1305_AEAD_256BYTES_ONLY_ENCRYPT`
|               12.85 |       77,820,632.29 |    0.4% |      0.01 | `CHACHA20_POLY1305_AEAD_64BYTES_ENCRYPT_DECRYPT`
|                6.63 |      150,833,608.84 |    1.7% |      0.01 | `CHACHA20_POLY1305_AEAD_64BYTES_ONLY_ENCRYPT`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|      315,561,738.00 |                3.17 |    4.2% |      3.49 | `ComplexMemPool`

|            ns/block |             block/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|        7,916,671.00 |              126.32 |    1.8% |      0.09 | `DeserializeAndCheckBlockTest`
|        6,464,441.00 |              154.69 |    1.3% |      0.07 | `DeserializeBlockTest`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|        9,867,024.00 |              101.35 |    1.5% |      0.11 | `DuplicateInputs`
|           13,953.68 |           71,665.67 |    0.4% |      0.01 | `EvictionProtection0Networks250Candidates`
|           10,696.34 |           93,489.91 |    0.6% |      0.01 | `EvictionProtection1Networks250Candidates`
|           17,890.98 |           55,894.08 |    2.0% |      0.01 | `EvictionProtection2Networks250Candidates`
|            3,844.74 |          260,095.62 |    1.8% |      0.01 | `EvictionProtection3Networks050Candidates`
|           11,772.28 |           84,945.34 |    2.0% |      0.01 | `EvictionProtection3Networks100Candidates`
|           26,802.80 |           37,309.53 |    4.6% |      0.01 | `EvictionProtection3Networks250Candidates`
|       27,870,722.00 |               35.88 |    2.5% |      0.30 | `ExpandDescriptor`
|                1.75 |      572,554,452.57 |    1.9% |      0.01 | `FastRandom_1bit`
|               10.98 |       91,115,876.28 |    3.6% |      0.01 | `FastRandom_32bit`
|          587,430.00 |            1,702.33 |    0.2% |      0.01 | `GCSBlockFilterGetHash`
|       16,112,011.00 |               62.07 |    3.1% |      0.18 | `GCSFilterConstruct`
|        2,536,229.00 |              394.29 |    0.7% |      0.03 | `GCSFilterDecode`
|            6,592.34 |          151,691.11 |    2.7% |      0.01 | `GCSFilterDecodeSkipCheck`
|          309,209.33 |            3,234.06 |    1.3% |      0.01 | `GCSFilterMatch`

|             ns/byte |              byte/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                3.44 |      290,324,974.28 |    1.0% |      0.04 | `HASH_1MB`
|                5.43 |      184,229,490.57 |    1.6% |      0.01 | `HASH_256BYTES`
|               11.24 |       88,975,627.91 |    0.8% |      0.01 | `HASH_64BYTES`
|                0.47 |    2,122,248,989.16 |    1.9% |      0.01 | `HexStrBench`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                8.39 |      119,141,241.61 |    3.5% |      0.01 | `LoggingNoCategory`
|               91.08 |       10,979,695.04 |    0.6% |      0.01 | `LoggingNoFile`
|            8,569.09 |          116,698.49 |    4.8% |      0.01 | `LoggingNoThreadNames`
|            8,128.28 |          123,027.32 |    1.9% |      0.01 | `LoggingYoCategory`
|            8,787.87 |          113,793.27 |    4.4% |      0.01 | `LoggingYoThreadNames`
|      103,709,116.00 |                9.64 |    4.6% |      1.15 | `MempoolCheck`
|           41,341.33 |           24,188.87 |    7.0% |      0.01 | :wavy_dash: `MempoolEviction` (Unstable with ~23.5 iters. Increase `minEpochIterations` to e.g. 235)

|             ns/leaf |              leaf/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              153.27 |        6,524,387.99 |    3.8% |      0.02 | `MerkleRoot`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|            6,740.57 |          148,355.48 |    0.3% |      0.01 | `MuHash`
|            5,699.77 |          175,445.53 |    1.3% |      0.01 | `MuHashDiv`
|            5,712.18 |          175,064.50 |    0.9% |      0.01 | `MuHashMul`
|            1,139.70 |          877,427.35 |    1.7% |      0.01 | `MuHashPrecompute`

|             ns/byte |              byte/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                0.95 |    1,047,989,126.09 |    1.7% |      0.01 | `POLY1305_1MB`
|                1.00 |    1,000,672,631.51 |    1.2% |      0.01 | `POLY1305_256BYTES`
|                1.18 |      844,267,678.75 |    0.6% |      0.01 | `POLY1305_64BYTES`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              247.25 |        4,044,487.07 |    0.5% |      0.01 | `PrePadded`
|               11.78 |       84,881,371.59 |    0.8% |      0.01 | `PrevectorClearNontrivial`
|               10.30 |       97,041,477.31 |    1.1% |      0.01 | `PrevectorClearTrivial`
|              483.98 |        2,066,194.68 |    0.6% |      0.01 | `PrevectorDeserializeNontrivial`
|               27.73 |       36,066,999.76 |    1.7% |      0.01 | `PrevectorDeserializeTrivial`
|                   - |                   - |       - |         - | :boom: `PrevectorDestructorNontrivial` (iterations overflow. Maybe your code got optimized away?)
|                   - |                   - |       - |         - | :boom: `PrevectorDestructorTrivial` (iterations overflow. Maybe your code got optimized away?)
|                5.88 |      170,127,668.87 |    0.9% |      0.01 | `PrevectorResizeNontrivial`
|                4.58 |      218,575,942.26 |    1.4% |      0.01 | `PrevectorResizeTrivial`

|             ns/byte |              byte/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                2.74 |      364,884,001.55 |    0.6% |      0.03 | `RIPEMD160`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              482.08 |        2,074,334.44 |    1.4% |      0.01 | `RegularPadded`
|              575.46 |        1,737,731.61 |    3.8% |      0.01 | `RollingBloom`
|           36,297.70 |           27,549.95 |    3.0% |      0.01 | `RollingBloomReset`
|       10,912,255.00 |               91.64 |    2.9% |      0.12 | `RpcMempool`

|             ns/byte |              byte/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|                2.07 |      482,841,036.66 |    1.9% |      0.02 | `SHA1`
|                3.44 |      291,058,112.66 |    0.8% |      0.04 | `SHA256`
|                2.16 |      462,721,222.65 |    2.5% |      0.01 | `SHA256D64_1024`
|                7.70 |      129,939,070.47 |    0.5% |      0.01 | `SHA256_32b`
|                4.32 |      231,372,382.98 |    2.0% |      0.05 | `SHA3_256_1M`
|                3.17 |      315,322,530.80 |    1.5% |      0.03 | `SHA512`

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|               31.30 |       31,949,692.30 |    1.1% |      0.01 | `SipHash_32b`
|               11.07 |       90,358,260.26 |    3.9% |      0.02 | `Trig`
|          114,252.56 |            8,752.54 |    1.0% |      0.01 | `VerifyNestedIfScript`
|          107,090.00 |            9,337.94 |    4.8% |      0.01 | `VerifyScriptBench`

kevkevinpal · 2022-12-23T02:22:50Z

.cirrus.yml

@@ -175,7 +175,7 @@ task:
    - ccache --show-stats
  check_script:
    - src\test_bitcoin.exe -l test_suite
-    - src\bench_bitcoin.exe --sanity-check > NUL
+    - src\bench_bitcoin.exe -sanity-check -priority-level=high


is there a reason why we changed this to only include -priority-level=high?

See #26158 (comment)

unrelated: It could make sense to have one CI task run all priorities

Ok. The last commit has been dropped.

unrelated: It could be set to run all priorities explicitly (and mentioned in the CI task name) to make it explicit and documented for devs.

aureleoules

tACK f1e8959. Ran as expected and is more practical than using an output redirection.

$ ./src/bench/bench_bitcoin -sanity-check -priority-level=high
Running with -sanity-check option, output is being suppressed as benchmark results will be useless.

…y-check…

hebasto added 2 commits November 10, 2022 16:26

bench: Suppress output when running with -sanity-check option

4dbcdf2

This change allows to simplify CI tests, and makes it easier to integrate the `bench_bitcoin` binary into CMake custom targets or commands, as `COMMAND` does not support output redirection

test: Drop no longer required bench output redirection

f1e8959

DrahtBot added the Tests label Nov 10, 2022

hebasto mentioned this pull request Nov 10, 2022

bench: add "priority level" to the benchmark framework #26158

Merged

shaavan reviewed Nov 12, 2022

View reviewed changes

DrahtBot mentioned this pull request Nov 12, 2022

build: Add CMake-based build system #25797

Closed

16 tasks

kevkevinpal reviewed Dec 23, 2022

View reviewed changes

hebasto force-pushed the 221110-bench branch from 4af665f to f1e8959 Compare December 23, 2022 14:26

aureleoules approved these changes Dec 29, 2022

View reviewed changes

maflcko merged commit b9028b2 into bitcoin:master Dec 29, 2022

hebasto deleted the 221110-bench branch December 29, 2022 10:45

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 29, 2022

Merge bitcoin#26481: bench: Suppress output when running with `-sanit…

e553ce3

…y-check…

bitcoin locked and limited conversation to collaborators Dec 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

bench: Suppress output when running with `-sanity-check` option #26481

bench: Suppress output when running with `-sanity-check` option #26481

Uh oh!

hebasto commented Nov 10, 2022

Uh oh!

DrahtBot commented Nov 12, 2022 •

edited

Loading

Uh oh!

shaavan left a comment

Uh oh!

kevkevinpal commented Dec 23, 2022

Uh oh!

kevkevinpal Dec 23, 2022

Uh oh!

hebasto Dec 23, 2022

Uh oh!

maflcko Dec 23, 2022

Uh oh!

hebasto Dec 23, 2022

Uh oh!

maflcko Dec 23, 2022

Uh oh!

aureleoules left a comment

Uh oh!

Uh oh!

bench: Suppress output when running with -sanity-check option #26481

bench: Suppress output when running with -sanity-check option #26481

Uh oh!

Conversation

hebasto commented Nov 10, 2022

Uh oh!

DrahtBot commented Nov 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

shaavan left a comment

Choose a reason for hiding this comment

Uh oh!

kevkevinpal commented Dec 23, 2022

Uh oh!

kevkevinpal Dec 23, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Dec 23, 2022

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 23, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Dec 23, 2022

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 23, 2022

Choose a reason for hiding this comment

Uh oh!

aureleoules left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bench: Suppress output when running with `-sanity-check` option #26481

bench: Suppress output when running with `-sanity-check` option #26481

DrahtBot commented Nov 12, 2022 •

edited

Loading