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

hebasto commented at 4:28 pm on November 10, 2022: member

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

4dbcdf26a3

test: Drop no longer required bench output redirection f1e89597c8

DrahtBot added the label Tests on Nov 10, 2022

DrahtBot commented at 0:25 am on November 12, 2022: contributor

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 commented at 7:59 am on November 12, 2022: contributor

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 commented at 1:59 am on December 23, 2022: contributor

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

  0|               ns/op |                op/s |    err% |     total | benchmark
  1|--------------------:|--------------------:|--------:|----------:|:----------
  2|       58,488,580.00 |               17.10 |    2.3% |      0.63 | `AddrManAdd`
  3|      144,473,637.00 |                6.92 |    6.3% |      1.57 | :wavy_dash: `AddrManAddThenGood` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
  4|          249,270.00 |            4,011.71 |    9.3% |      0.01 | :wavy_dash: `AddrManGetAddr` (Unstable with ~2.7 iters. Increase `minEpochIterations` to e.g. 27)
  5|              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)
  6|          447,008.50 |            2,237.09 |    1.8% |      0.01 | `AssembleBlock`
  7
  8|             ns/byte |              byte/s |    err% |     total | benchmark
  9|--------------------:|--------------------:|--------:|----------:|:----------
 10|              111.39 |        8,977,617.86 |    1.4% |      0.01 | `Base58CheckEncode`
 11|               25.14 |       39,771,942.56 |    2.0% |      0.01 | `Base58Decode`
 12|               83.38 |       11,993,484.31 |    1.5% |      0.01 | `Base58Encode`
 13|               13.88 |       72,048,351.15 |    4.3% |      0.01 | `Bech32Decode`
 14|               32.47 |       30,800,157.58 |    2.0% |      0.01 | `Bech32Encode`
 15
 16|               ns/op |                op/s |    err% |     total | benchmark
 17|--------------------:|--------------------:|--------:|----------:|:----------
 18|              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)
 19|               35.21 |       28,402,277.93 |    1.2% |      0.01 | `BenchTimeDeprecated`
 20|               34.60 |       28,902,932.85 |    2.0% |      0.01 | `BenchTimeMillis`
 21|               35.19 |       28,416,030.97 |    1.2% |      0.01 | `BenchTimeMillisSys`
 22|                2.36 |      423,272,811.99 |    1.0% |      0.01 | `BenchTimeMock`
 23|      124,648,173.00 |                8.02 |    3.5% |      1.39 | `BlockToJsonVerbose`
 24|       56,973,872.00 |               17.55 |    1.4% |      0.63 | `BlockToJsonVerboseWrite`
 25
 26|              ns/job |               job/s |    err% |     total | benchmark
 27|--------------------:|--------------------:|--------:|----------:|:----------
 28|              844.28 |        1,184,435.76 |    5.2% |      0.31 | :wavy_dash: `CCheckQueueSpeedPrevectorJob` (Unstable with ~10.9 iters. Increase `minEpochIterations` to e.g. 109)
 29
 30|               ns/op |                op/s |    err% |     total | benchmark
 31|--------------------:|--------------------:|--------:|----------:|:----------
 32|              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)
 33
 34|             ns/byte |              byte/s |    err% |     total | benchmark
 35|--------------------:|--------------------:|--------:|----------:|:----------
 36|                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)
 37|                2.00 |      499,795,120.75 |    3.9% |      0.01 | `CHACHA20_256BYTES`
 38|                2.15 |      464,722,164.36 |    0.2% |      0.01 | `CHACHA20_64BYTES`
 39|                6.00 |      166,640,656.61 |    1.8% |      0.07 | `CHACHA20_POLY1305_AEAD_1MB_ENCRYPT_DECRYPT`
 40|                2.98 |      335,652,695.54 |    0.7% |      0.03 | `CHACHA20_POLY1305_AEAD_1MB_ONLY_ENCRYPT`
 41|                7.89 |      126,815,704.95 |    3.1% |      0.01 | `CHACHA20_POLY1305_AEAD_256BYTES_ENCRYPT_DECRYPT`
 42|                3.86 |      258,855,192.23 |    1.2% |      0.01 | `CHACHA20_POLY1305_AEAD_256BYTES_ONLY_ENCRYPT`
 43|               12.85 |       77,820,632.29 |    0.4% |      0.01 | `CHACHA20_POLY1305_AEAD_64BYTES_ENCRYPT_DECRYPT`
 44|                6.63 |      150,833,608.84 |    1.7% |      0.01 | `CHACHA20_POLY1305_AEAD_64BYTES_ONLY_ENCRYPT`
 45
 46|               ns/op |                op/s |    err% |     total | benchmark
 47|--------------------:|--------------------:|--------:|----------:|:----------
 48|      315,561,738.00 |                3.17 |    4.2% |      3.49 | `ComplexMemPool`
 49
 50|            ns/block |             block/s |    err% |     total | benchmark
 51|--------------------:|--------------------:|--------:|----------:|:----------
 52|        7,916,671.00 |              126.32 |    1.8% |      0.09 | `DeserializeAndCheckBlockTest`
 53|        6,464,441.00 |              154.69 |    1.3% |      0.07 | `DeserializeBlockTest`
 54
 55|               ns/op |                op/s |    err% |     total | benchmark
 56|--------------------:|--------------------:|--------:|----------:|:----------
 57|        9,867,024.00 |              101.35 |    1.5% |      0.11 | `DuplicateInputs`
 58|           13,953.68 |           71,665.67 |    0.4% |      0.01 | `EvictionProtection0Networks250Candidates`
 59|           10,696.34 |           93,489.91 |    0.6% |      0.01 | `EvictionProtection1Networks250Candidates`
 60|           17,890.98 |           55,894.08 |    2.0% |      0.01 | `EvictionProtection2Networks250Candidates`
 61|            3,844.74 |          260,095.62 |    1.8% |      0.01 | `EvictionProtection3Networks050Candidates`
 62|           11,772.28 |           84,945.34 |    2.0% |      0.01 | `EvictionProtection3Networks100Candidates`
 63|           26,802.80 |           37,309.53 |    4.6% |      0.01 | `EvictionProtection3Networks250Candidates`
 64|       27,870,722.00 |               35.88 |    2.5% |      0.30 | `ExpandDescriptor`
 65|                1.75 |      572,554,452.57 |    1.9% |      0.01 | `FastRandom_1bit`
 66|               10.98 |       91,115,876.28 |    3.6% |      0.01 | `FastRandom_32bit`
 67|          587,430.00 |            1,702.33 |    0.2% |      0.01 | `GCSBlockFilterGetHash`
 68|       16,112,011.00 |               62.07 |    3.1% |      0.18 | `GCSFilterConstruct`
 69|        2,536,229.00 |              394.29 |    0.7% |      0.03 | `GCSFilterDecode`
 70|            6,592.34 |          151,691.11 |    2.7% |      0.01 | `GCSFilterDecodeSkipCheck`
 71|          309,209.33 |            3,234.06 |    1.3% |      0.01 | `GCSFilterMatch`
 72
 73|             ns/byte |              byte/s |    err% |     total | benchmark
 74|--------------------:|--------------------:|--------:|----------:|:----------
 75|                3.44 |      290,324,974.28 |    1.0% |      0.04 | `HASH_1MB`
 76|                5.43 |      184,229,490.57 |    1.6% |      0.01 | `HASH_256BYTES`
 77|               11.24 |       88,975,627.91 |    0.8% |      0.01 | `HASH_64BYTES`
 78|                0.47 |    2,122,248,989.16 |    1.9% |      0.01 | `HexStrBench`
 79
 80|               ns/op |                op/s |    err% |     total | benchmark
 81|--------------------:|--------------------:|--------:|----------:|:----------
 82|                8.39 |      119,141,241.61 |    3.5% |      0.01 | `LoggingNoCategory`
 83|               91.08 |       10,979,695.04 |    0.6% |      0.01 | `LoggingNoFile`
 84|            8,569.09 |          116,698.49 |    4.8% |      0.01 | `LoggingNoThreadNames`
 85|            8,128.28 |          123,027.32 |    1.9% |      0.01 | `LoggingYoCategory`
 86|            8,787.87 |          113,793.27 |    4.4% |      0.01 | `LoggingYoThreadNames`
 87|      103,709,116.00 |                9.64 |    4.6% |      1.15 | `MempoolCheck`
 88|           41,341.33 |           24,188.87 |    7.0% |      0.01 | :wavy_dash: `MempoolEviction` (Unstable with ~23.5 iters. Increase `minEpochIterations` to e.g. 235)
 89
 90|             ns/leaf |              leaf/s |    err% |     total | benchmark
 91|--------------------:|--------------------:|--------:|----------:|:----------
 92|              153.27 |        6,524,387.99 |    3.8% |      0.02 | `MerkleRoot`
 93
 94|               ns/op |                op/s |    err% |     total | benchmark
 95|--------------------:|--------------------:|--------:|----------:|:----------
 96|            6,740.57 |          148,355.48 |    0.3% |      0.01 | `MuHash`
 97|            5,699.77 |          175,445.53 |    1.3% |      0.01 | `MuHashDiv`
 98|            5,712.18 |          175,064.50 |    0.9% |      0.01 | `MuHashMul`
 99|            1,139.70 |          877,427.35 |    1.7% |      0.01 | `MuHashPrecompute`
100
101|             ns/byte |              byte/s |    err% |     total | benchmark
102|--------------------:|--------------------:|--------:|----------:|:----------
103|                0.95 |    1,047,989,126.09 |    1.7% |      0.01 | `POLY1305_1MB`
104|                1.00 |    1,000,672,631.51 |    1.2% |      0.01 | `POLY1305_256BYTES`
105|                1.18 |      844,267,678.75 |    0.6% |      0.01 | `POLY1305_64BYTES`
106
107|               ns/op |                op/s |    err% |     total | benchmark
108|--------------------:|--------------------:|--------:|----------:|:----------
109|              247.25 |        4,044,487.07 |    0.5% |      0.01 | `PrePadded`
110|               11.78 |       84,881,371.59 |    0.8% |      0.01 | `PrevectorClearNontrivial`
111|               10.30 |       97,041,477.31 |    1.1% |      0.01 | `PrevectorClearTrivial`
112|              483.98 |        2,066,194.68 |    0.6% |      0.01 | `PrevectorDeserializeNontrivial`
113|               27.73 |       36,066,999.76 |    1.7% |      0.01 | `PrevectorDeserializeTrivial`
114|                   - |                   - |       - |         - | :boom: `PrevectorDestructorNontrivial` (iterations overflow. Maybe your code got optimized away?)
115|                   - |                   - |       - |         - | :boom: `PrevectorDestructorTrivial` (iterations overflow. Maybe your code got optimized away?)
116|                5.88 |      170,127,668.87 |    0.9% |      0.01 | `PrevectorResizeNontrivial`
117|                4.58 |      218,575,942.26 |    1.4% |      0.01 | `PrevectorResizeTrivial`
118
119|             ns/byte |              byte/s |    err% |     total | benchmark
120|--------------------:|--------------------:|--------:|----------:|:----------
121|                2.74 |      364,884,001.55 |    0.6% |      0.03 | `RIPEMD160`
122
123|               ns/op |                op/s |    err% |     total | benchmark
124|--------------------:|--------------------:|--------:|----------:|:----------
125|              482.08 |        2,074,334.44 |    1.4% |      0.01 | `RegularPadded`
126|              575.46 |        1,737,731.61 |    3.8% |      0.01 | `RollingBloom`
127|           36,297.70 |           27,549.95 |    3.0% |      0.01 | `RollingBloomReset`
128|       10,912,255.00 |               91.64 |    2.9% |      0.12 | `RpcMempool`
129
130|             ns/byte |              byte/s |    err% |     total | benchmark
131|--------------------:|--------------------:|--------:|----------:|:----------
132|                2.07 |      482,841,036.66 |    1.9% |      0.02 | `SHA1`
133|                3.44 |      291,058,112.66 |    0.8% |      0.04 | `SHA256`
134|                2.16 |      462,721,222.65 |    2.5% |      0.01 | `SHA256D64_1024`
135|                7.70 |      129,939,070.47 |    0.5% |      0.01 | `SHA256_32b`
136|                4.32 |      231,372,382.98 |    2.0% |      0.05 | `SHA3_256_1M`
137|                3.17 |      315,322,530.80 |    1.5% |      0.03 | `SHA512`
138
139|               ns/op |                op/s |    err% |     total | benchmark
140|--------------------:|--------------------:|--------:|----------:|:----------
141|               31.30 |       31,949,692.30 |    1.1% |      0.01 | `SipHash_32b`
142|               11.07 |       90,358,260.26 |    3.9% |      0.02 | `Trig`
143|          114,252.56 |            8,752.54 |    1.0% |      0.01 | `VerifyNestedIfScript`
144|          107,090.00 |            9,337.94 |    4.8% |      0.01 | `VerifyScriptBench`

in .cirrus.yml:178 in 4af665f840 outdated

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

kevkevinpal commented at 2:22 am on December 23, 2022:

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

hebasto commented at 1:47 pm on December 23, 2022:

See #26158 (review)

maflcko commented at 2:13 pm on December 23, 2022:

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

hebasto commented at 2:26 pm on December 23, 2022:

Ok. The last commit has been dropped.

maflcko commented at 2:37 pm on December 23, 2022:

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.

hebasto force-pushed on Dec 23, 2022

aureleoules approved

aureleoules commented at 10:24 am on December 29, 2022: member

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

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

maflcko merged this on Dec 29, 2022

maflcko closed this on Dec 29, 2022

hebasto deleted the branch on Dec 29, 2022

sidhujag referenced this in commit e553ce3034 on Dec 29, 2022

bitcoin locked this on Dec 29, 2023

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

Reviews

Conflicts

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