forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add DSHA256 and X11 benchmarks, refactor #1925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ks to group them together DSHA256 and X11 also have additional tests for data from 32 to 2048 bytes (for comparison, in steps)
codablock
approved these changes
Feb 12, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Also closes: #1806
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…ks to group them together (dashpay#1925) DSHA256 and X11 also have additional tests for data from 32 to 2048 bytes (for comparison, in steps)
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 28, 2019
…ks to group them together (dashpay#1925) DSHA256 and X11 also have additional tests for data from 32 to 2048 bytes (for comparison, in steps)
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Mar 2, 2019
…ks to group them together (dashpay#1925) DSHA256 and X11 also have additional tests for data from 32 to 2048 bytes (for comparison, in steps)
PastaPastaPasta
added a commit
that referenced
this pull request
Jul 17, 2025
, bitcoin#27598, bitcoin#29180, bitcoin#29407, bitcoin#29528, bitcoin#28893, bitcoin#29834, partial bitcoin#26158, bitcoin#25200, bitcoin#24322 (auxiliary backports: part 25) 2bb8e5b chore: resolve nit from review (Kittywhiskers Van Gogh) 61bba13 fix: tweak bls/bls_dkg benchmarks to work faster in `-sanity-check` mode (UdjinM6) 0eff28c merge bitcoin#29834: Change MAC_OSX macro to __APPLE__ in crypto (Kittywhiskers Van Gogh) 76cef17 merge bitcoin#28893: Fix SSE4.1-related issues (Kittywhiskers Van Gogh) 16209a8 merge bitcoin#29528: move sha256_sse4 into libbitcoin_crypto_base (Kittywhiskers Van Gogh) ec1f492 merge bitcoin#29407: remove confusing and inconsistent disable-asm option (Kittywhiskers Van Gogh) 9440c18 merge bitcoin#29180: remove use of BUILD_BITCOIN_INTERNAL macro in sha256 (Kittywhiskers Van Gogh) 68f6bea fix: place `LIBTEST_*` before `LIBBITCOIN_*` for fuzzing binaries (Kittywhiskers Van Gogh) a3c11b6 partial bitcoin#24322: Introduce initial `libbitcoinkernel` (Kittywhiskers Van Gogh) 9f981dd merge bitcoin#27598: Add SHA256 implementation specific benchmarks (Kittywhiskers Van Gogh) 306fa9a chore: realign function names in `crypto_hash` bench with upstream (Kittywhiskers Van Gogh) 3bdc15a chore: replace leftover `minEpochIterations` from `crypto_hash` benches (Kittywhiskers Van Gogh) f300277 chore: drop `minEpochIterations(10)` from `crypto_hash` bench functions (Kittywhiskers Van Gogh) c8f0d54 chore: move Dash-specific tests downward, sort like upstream (Kittywhiskers Van Gogh) fda951f merge bitcoin#26481: Suppress output when running with `-sanity-check` option (Kittywhiskers Van Gogh) a5718a0 partial bitcoin#25200: Fix spelling errors identified by codespell in comments (Kittywhiskers Van Gogh) 58a6248 partial bitcoin#26158: add "priority level" to the benchmark framework (Kittywhiskers Van Gogh) 160b3fc merge bitcoin#25107: Add `--sanity-check` flag, use it in `make check` (Kittywhiskers Van Gogh) 2ad60b3 fix: use atomic in `BLS_Verify_BatchedParallel` to avoid data race (Kittywhiskers Van Gogh) 24ad076 fix: ensure that globals are set and checks present to avoid bench errs (Kittywhiskers Van Gogh) 1c54997 trivial: re-enable benchmarks for `make check` (Kittywhiskers Van Gogh) 2a0df15 merge bitcoin#25058: Move output script RPCs to separate file, rename misc.cpp (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * `make check` upstream includes a dry run of the benchmarking suite, this behavior was introduced in [bitcoin#14092](bitcoin#14092) (a557022) but was disabled for Dash Core as our benchmarking suite is substantially heavier. Unfortunately, as a side effect of that, failures in the benchmark suite go unnoticed by CI (see e507a51 for one such fix). With the introduction of [bitcoin#25107](bitcoin#25107) and [bitcoin#26158](bitcoin#26158), the sanity checks require significantly less overhead and should be suitable for reintroduction into the `make check` rotation. * The sanity check caught failures in the test suite ([build](https://github.com/dashpay/dash/actions/runs/16291896463/job/46003746200#step:10:206)) due to missing initialization or insufficient checking and they have been remedied in this pull request. An example is available below. <details> <summary>Debugger stacktrace</summary> ``` #0 StatsdClient::send<unsigned long> (this=0x0, key="CheckBlock_us", value=2847, type="ms", sample_rate=1) at stats/client.cpp:108 #1 0x0000555555c3603f in StatsdClient::timing (this=0x0, key="CheckBlock_us", ms=2847, sample_rate=1) at stats/client.cpp:100 #2 0x0000555555ca0b51 in CheckBlock (block=..., state=..., consensusParams=..., fCheckPOW=true, fCheckMerkleRoot=true) at validation.cpp:3932 #3 0x000055555560cca5 in DeserializeAndCheckBlockTest(ankerl::nanobench::Bench&)::$_0::operator()() const (this=0x7fffffffd120) at bench/checkblock.cpp:47 #4 ankerl::nanobench::Bench::run<DeserializeAndCheckBlockTest(ankerl::nanobench::Bench&)::$_0>(DeserializeAndCheckBlockTest(ankerl::nanobench::Bench&)::$_0&&) (this=0x7fffffffd430, op=...) at ./bench/nanobench.h:1221 #5 0x000055555560c74b in DeserializeAndCheckBlockTest (bench=...) at bench/checkblock.cpp:40 #6 0x00005555555e2f4a in std::function<void (ankerl::nanobench::Bench&)>::operator()(ankerl::nanobench::Bench&) const (this=0x555556942a90, __args=...) at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:591 #7 benchmark::BenchRunner::RunAll (args=...) at bench/bench.cpp:120 #8 0x00005555555de5f4 in main (argc=<optimized out>, argv=<optimized out>) at bench/bench_bitcoin.cpp:134 ``` </details> * The benchmark `BLS_Verify_BatchedParallel` also has a data race that was spotted courtesy of `make check` ([build](https://github.com/dashpay/dash/actions/runs/16295305205/job/46015360773#step:10:183)) that has been resolved by using an atomic. * To allow backporting [bitcoin#27598](bitcoin#27598), the naming scheme changes in `src/bench/crypto_hash.cpp` introduced by [dash#1925](#1925) have been effectively reverted and Dash-specific benchmarks have been moved downward to mitigate merge conflict issues. * Similarly, changes to minimum iteration and batching by input size were made to realign with upstream. * The order of libraries for the linker had to be rearranged as the linker reads dependencies from left to right (i.e. top to bottom), which makes them ordering sensitive. Without this, the linker complains visibly (see below). <details> <summary>Linker errors:</summary> ``` $ make -j10 [...] AR libbitcoin_node.a CXXLD dashd CXXLD test/test_dash CXXLD qt/dash-qt CXXLD test/fuzz/fuzz CXXLD qt/test/test_dash-qt CXXLD bench/bench_dash /usr/bin/ld: libtest_util.a(libtest_util_a-setup_common.o): in function `DashChainstateSetup(ChainstateManager&, node::NodeContext&, bool, bool, Consensus::Params const&)': /src/dash/dash/src/test/util/setup_common.cpp:128: undefined reference to `node::DashChainstateSetup(ChainstateManager&, CGovernanceManager&, CMasternodeMetaMan&, CMasternodeSync&, CSporkManager&, std::unique_ptr<CActiveMasternodeManager, std::default_delete<CActiveMasternodeManager> >&, std::unique_ptr<CChainstateHelper, std::default_delete<CChainstateHelper> >&, std::unique_ptr<CCreditPoolManager, std::default_delete<CCreditPoolManager> >&, std::unique_ptr<CDeterministicMNManager, std::default_delete<CDeterministicMNManager> >&, std::unique_ptr<CEvoDB, std::default_delete<CEvoDB> >&, std::unique_ptr<CMNHFManager, std::default_delete<CMNHFManager> >&, std::unique_ptr<LLMQContext, std::default_delete<LLMQContext> >&, CTxMemPool*, bool, bool, Consensus::Params const&)' /usr/bin/ld: libtest_util.a(libtest_util_a-setup_common.o): in function `DashChainstateSetupClose(node::NodeContext&)': /src/dash/dash/src/test/util/setup_common.cpp:136: undefined reference to `node::DashChainstateSetupClose(std::unique_ptr<CChainstateHelper, std::default_delete<CChainstateHelper> >&, std::unique_ptr<CCreditPoolManager, std::default_delete<CCreditPoolManager> >&, std::unique_ptr<CDeterministicMNManager, std::default_delete<CDeterministicMNManager> >&, std::unique_ptr<CMNHFManager, std::default_delete<CMNHFManager> >&, std::unique_ptr<LLMQContext, std::default_delete<LLMQContext> >&, CTxMemPool*)' /usr/bin/ld: libtest_util.a(libtest_util_a-setup_common.o): in function `BasicTestingSetup::BasicTestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&)': [...] clang: error: linker command failed with exit code 1 (use -v to see invocation) make[2]: *** [Makefile:7723: test/fuzz/fuzz] Error 1 make[2]: *** Waiting for unfinished jobs.... make[2]: Leaving directory '/src/dash/dash/src' make[1]: *** [Makefile:20430: all-recursive] Error 1 make[1]: Leaving directory '/src/dash/dash/src' make: *** [Makefile:796: all-recursive] Error 1 ``` </details> ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 2bb8e5b Tree-SHA512: 1771f6af9d7a4da9f5417dc60fa0a69f7f1adb1c366ab458d9c2d572c2924ac03fd2c4b9e00cda8a63022cbc193b6b262349c5eab79654a1d0006e5e65327340
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add DSHA256 and X11 benchmarks. Also refactor names of other algo benchmarks to group them together.
DSHA256 and X11 also have additional tests for data from 32 to 2048 bytes (for comparison, in steps).
Looks like this now:
EDIT: closes #1806 as was reminded below :)