From da1d3f2654838377341857a7cf35f7759361dd84 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Mon, 1 Aug 2022 11:04:58 +0200 Subject: [PATCH 01/21] Merge bitcoin/bitcoin#25663: tracing: do not use `coin` after move in `CCoinsViewCache::AddCoin` f8e228476f54b36beacc3fd09038dfeebdac6b3c tracing: do not use `coin` after move in `CCoinsViewCache::AddCoin` (Seibart Nedor) Pull request description: This is fix for https://github.com/bitcoin/bitcoin/issues/25640. ACKs for top commit: 0xB10C: ACK f8e228476f54b36beacc3fd09038dfeebdac6b3c Tree-SHA512: e7643ac8e6b6247aaf250f44572c4b458da4aea030ac0268227564e6857200e9c23efe325cfc535f46498cbeccaf46301551efeeb54b062f71d2dcf1ffe71fb8 --- src/coins.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index e0e455d632297..b81a005da6bc5 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -102,9 +102,9 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi TRACE5(utxocache, add, outpoint.hash.data(), (uint32_t)outpoint.n, - (uint32_t)coin.nHeight, - (int64_t)coin.out.nValue, - (bool)coin.IsCoinBase()); + (uint32_t)it->second.coin.nHeight, + (int64_t)it->second.coin.out.nValue, + (bool)it->second.coin.IsCoinBase()); } void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin) { From 3be81a2d4c2a383fe3291e790562dedf9b4d8aae Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 31 Aug 2022 11:19:55 -0400 Subject: [PATCH 02/21] Merge bitcoin/bitcoin#25915: test: Fix wallet_balance intermittent issue fae5bd920007c33de0b794bf2b2d698bfccc12ee test: Fix wallet_balance intermittent issue (MacroFake) Pull request description: Diff to reproduce: ```diff index d2ed97ca76..25cc2d5734 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -265,7 +265,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].invalidateblock(block_reorg) self.nodes[1].invalidateblock(block_reorg) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted - self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY, sync_fun=self.no_op) + self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted # Now confirm tx_orig ``` Example in CI: ``` test 2022-08-24T10:09:22.486000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 133, in main self.run_test() File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/wallet_balance.py", line 269, in run_test assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/util.py", line 56, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not(98.85983340 == 0) ``` https://cirrus-ci.com/task/4981266251513856?logs=ci#L3269 ACKs for top commit: achow101: ACK fae5bd920007c33de0b794bf2b2d698bfccc12ee w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25915/commits/fae5bd920007c33de0b794bf2b2d698bfccc12ee Tree-SHA512: 470f366720615c4a9326ec4c581fff569ecce9877f9134bb1975ec3d6f1d13a6403051418a91a80b2a86de617f43e539ec11bbf4f1713d0354d5b0ab98d22437 --- test/functional/wallet_balance.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 317b48ea9f0e2..242bd76bfc177 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -266,7 +266,6 @@ def test_balances(*, fee_node_1=0): self.nodes[1].invalidateblock(block_reorg) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY, sync_fun=self.no_op) - assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted # Now confirm tx_orig self.restart_node(1, ['-persistmempool=0', '-checklevel=0']) From 459425776ce0c8b983ee25c156dfccacabfd2535 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Mon, 3 Oct 2022 11:36:05 +0200 Subject: [PATCH 03/21] Merge bitcoin/bitcoin#26229: test: Use proper Boost macros instead of assertions 5c9a27a46f4b07f872f850f7e918c10a33595cfd test: Use proper Boost macros instead of assertions (Hennadii Stepanov) Pull request description: On the master branch: ``` $ src/test/test_bitcoin -l test_suite -t banman_tests Running 1 test case... ... Test case banman_tests/file did not check any assertions ... ``` This PR suggests to use proper Boost [macros](https://www.boost.org/doc/libs/1_80_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref.html). Top commit has no ACKs. Tree-SHA512: e0c8e5e6371acd0e0a80070fffdf1445f264c62499f8d9811822994c89735a913c18c8ed730495578400abdd93d2d500345504f2a9246401d53fb2f9f71be8c5 --- src/test/banman_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/banman_tests.cpp b/src/test/banman_tests.cpp index 27ce9ad63825c..ecf60834ce848 100644 --- a/src/test/banman_tests.cpp +++ b/src/test/banman_tests.cpp @@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(file) " { \"version\": 1, \"ban_created\": 0, \"banned_until\": 778, \"address\": \"1.0.0.0/8\" }" "] }", }; - assert(WriteBinaryFile(banlist_path + ".json", entries_write)); + BOOST_REQUIRE(WriteBinaryFile(banlist_path + ".json", entries_write)); { // The invalid entries will be dropped, but the valid one remains ASSERT_DEBUG_LOG("Dropping entry with unparseable address or subnet (aaaaaaaaa) from ban list"); @@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(file) BanMan banman{banlist_path, /*client_interface=*/nullptr, /*default_ban_time=*/0}; banmap_t entries_read; banman.GetBanned(entries_read); - assert(entries_read.size() == 1); + BOOST_CHECK_EQUAL(entries_read.size(), 1); } } } From 5f7885956235f9406a98806fe9671ae0f9ee52d7 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 3 Nov 2022 10:16:27 +0000 Subject: [PATCH 04/21] Merge bitcoin/bitcoin#25248: refactor: Add LIFETIMEBOUND / -Wdangling-gsl to Assert() fa3ea81c3e7d962715788ab5525958a532c51414 refactor: Add LIFETIMEBOUND / -Wdangling-gsl to Assert() (MacroFake) Pull request description: Currently compiles clean, but I think it may still be useful. Can be tested by adding an `&`: ```diff diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 5766fff92d..300c1ec60f 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -125,7 +125,7 @@ BOOST_AUTO_TEST_CASE(util_check) // Check -Wdangling-gsl does not trigger when copying the int. (It would // trigger on "const int&") - const int nine{*Assert(std::optional{9})}; + const int& nine{*Assert(std::optional{9})}; BOOST_CHECK_EQUAL(9, nine); } ``` Output: ``` test/util_tests.cpp:128:29: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl] const int& nine{*Assert(std::optional{9})}; ^~~~~~~~~~~~~~~~~~~~~ ./util/check.h:75:50: note: expanded from macro 'Assert' #define Assert(val) inline_assertion_check(val, __FILE__, __LINE__, __func__, #val) ^~~ 1 warning generated. ACKs for top commit: jonatack: ACK fa3ea81c3e7d962715788ab5525958a532c51414 theuni: ACK fa3ea81c3e7d962715788ab5525958a532c51414 Tree-SHA512: 17dea4d75f2ee2bf6e1b6a6f6d8f439711c777df0390574e8d8edb6ac9ee807a135341e4439050bd6a15ecc4097a1ba9a7ab15d27541ebf70a4e081fa6871877 --- src/test/util_tests.cpp | 5 +++++ src/util/check.h | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index f9897de32664c..68b6d1519af3a 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -123,6 +123,11 @@ BOOST_AUTO_TEST_CASE(util_check) // Check nested Asserts BOOST_CHECK_EQUAL(Assert((Assert(x).test() ? 3 : 0)), 3); + + // Check -Wdangling-gsl does not trigger when copying the int. (It would + // trigger on "const int&") + const int nine{*Assert(std::optional{9})}; + BOOST_CHECK_EQUAL(9, nine); } BOOST_AUTO_TEST_CASE(util_criticalsection) diff --git a/src/util/check.h b/src/util/check.h index 247d89bdb6e3f..ac16061f68dee 100644 --- a/src/util/check.h +++ b/src/util/check.h @@ -9,6 +9,7 @@ #include #endif +#include #include #include @@ -24,7 +25,7 @@ class NonFatalCheckError : public std::runtime_error /** Helper for CHECK_NONFATAL() */ template -T&& inline_check_non_fatal(T&& val, const char* file, int line, const char* func, const char* assertion) +T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion) { if (!(val)) { throw NonFatalCheckError( @@ -57,7 +58,7 @@ void assertion_fail(const char* file, int line, const char* func, const char* as /** Helper for Assert()/Assume() */ template -T&& inline_assertion_check(T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) +T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion) { if constexpr (IS_ASSERT #ifdef ABORT_ON_FAILED_ASSUME From 3261092f8506c2d274d859c9d41cfa2c7bb3e37f Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 22 Nov 2022 10:52:17 +0000 Subject: [PATCH 05/21] Merge bitcoin/bitcoin#26520: doc: test: update/fix TestShell example instructions 31d0067f8bccd6b090bf88bad8472bb5cb1f20a4 doc: test: update/fix TestShell example instructions (Sebastian Falbesoner) Pull request description: This PR tackles two issues in the TestShell documentation: - add missing instruction for creating a wallet prior to the `getnewaddress` call (needed as there is no default wallet created anymore since v0.21) - fix `generatetoaddress` call syntax (the scripted-diff in commit fa0b916971e5bc23ad6396831940a2899ca05402 only worked for tests using `BitcoinTestFramework`) ACKs for top commit: fanquake: ACK 31d0067f8bccd6b090bf88bad8472bb5cb1f20a4 - current instructions don't work. These do. Tree-SHA512: d2b7808a06892ad16728cb2b6d4a72b255ad711d27fe98b1de562f80444e7bb25d73296abdde4308162fe3be702864e2f7b7dbbbb000fe54c709951c09e6c730 --- test/functional/test-shell.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/functional/test-shell.md b/test/functional/test-shell.md index 78737509cb749..80f4e88109105 100644 --- a/test/functional/test-shell.md +++ b/test/functional/test-shell.md @@ -93,8 +93,10 @@ We now let the first node generate 101 regtest blocks, and direct the coinbase rewards to a wallet address owned by the mining node. ``` +>>> test.nodes[0].createwallet('default') +{'name': 'default', 'warning': 'Empty string given as passphrase, wallet will not be encrypted.'} >>> address = test.nodes[0].getnewaddress() ->>> test.self.generatetoaddress(nodes[0], 101, address) +>>> test.generatetoaddress(test.nodes[0], 101, address) ['2b98dd0044aae6f1cca7f88a0acf366a4bfe053c7f7b00da3c0d115f03d67efb', ... ``` Since the two nodes are both initialized by default to establish an outbound From 66a3981a7a0cf8f1c40f9ac4a31ce6bb508a04a7 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 8 Dec 2022 16:41:31 +0000 Subject: [PATCH 06/21] Merge bitcoin/bitcoin#24279: build: Make `$(package)_*_env` available to all `$(package)_*_cmds` affbf58a1e52a8e60c830be6a9e0347e0ff4c28e build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3c976572883bbf7f386bc88e2610dc1a58 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564eca8a688f39c75540877ec3bdfdde766b1) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin/bitcoin#22552. Another [bug](https://github.com/bitcoin/bitcoin/issues/22719) in the depends build system has already become a problem at least two times in the past (https://github.com/bitcoin/bitcoin/pull/16883#issuecomment-683817472 and https://github.com/bitcoin/bitcoin/pull/24134). Each time the problem was solved with other means. The initial [solution](https://github.com/bitcoin/bitcoin/pull/19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](https://github.com/bitcoin/bitcoin/pull/24134). The bug is well described in bitcoin/bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin/bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58a1e52a8e60c830be6a9e0347e0ff4c28e Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b --- depends/funcs.mk | 13 +++++++------ depends/packages/qt.mk | 4 +--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/depends/funcs.mk b/depends/funcs.mk index e76a69ffce8db..7bc38b09970ec 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -87,9 +87,9 @@ $(1)_download_path_fixed=$(subst :,\:,$$($(1)_download_path)) $(1)_fetch_cmds ?= $(call fetch_file,$(1),$(subst \:,:,$$($(1)_download_path_fixed)),$$($(1)_download_file),$($(1)_file_name),$($(1)_sha256_hash)) $(1)_extract_cmds ?= mkdir -p $$($(1)_extract_dir) && echo "$$($(1)_sha256_hash) $$($(1)_source)" > $$($(1)_extract_dir)/.$$($(1)_file_name).hash && $(build_SHA256SUM) -c $$($(1)_extract_dir)/.$$($(1)_file_name).hash && $(build_TAR) --no-same-owner --strip-components=1 -xf $$($(1)_source) $(1)_preprocess_cmds ?= true -$(1)_build_cmds ?= -$(1)_config_cmds ?= -$(1)_stage_cmds ?= +$(1)_build_cmds ?= true +$(1)_config_cmds ?= true +$(1)_stage_cmds ?= true $(1)_set_vars ?= @@ -137,6 +137,7 @@ $(1)_config_env+=$($(1)_config_env_$(host_arch)_$(host_os)) $($(1)_config_env_$( $(1)_config_env+=PKG_CONFIG_LIBDIR=$($($(1)_type)_prefix)/lib/pkgconfig $(1)_config_env+=PKG_CONFIG_PATH=$($($(1)_type)_prefix)/share/pkgconfig +$(1)_config_env+=PKG_CONFIG_SYSROOT_DIR=/ $(1)_config_env+=CMAKE_MODULE_PATH=$($($(1)_type)_prefix)/lib/cmake $(1)_config_env+=PATH=$(build_prefix)/bin:$(PATH) $(1)_build_env+=PATH=$(build_prefix)/bin:$(PATH) @@ -221,17 +222,17 @@ $($(1)_configured): | $($(1)_dependencies) $($(1)_preprocessed) echo Configuring $(1)... rm -rf $(host_prefix); mkdir -p $(host_prefix)/lib; cd $(host_prefix); $(foreach package,$($(1)_all_dependencies), $(build_TAR) --no-same-owner -xf $($(package)_cached); ) mkdir -p $$(@D) - +{ cd $$(@D); $($(1)_config_env) $($(1)_config_cmds); } $$($(1)_logging) + +{ cd $$(@D); export $($(1)_config_env); $($(1)_config_cmds); } $$($(1)_logging) touch $$@ $($(1)_built): | $($(1)_configured) echo Building $(1)... mkdir -p $$(@D) - +{ cd $$(@D); $($(1)_build_env) $($(1)_build_cmds); } $$($(1)_logging) + +{ cd $$(@D); export $($(1)_build_env); $($(1)_build_cmds); } $$($(1)_logging) touch $$@ $($(1)_staged): | $($(1)_built) echo Staging $(1)... mkdir -p $($(1)_staging_dir)/$(host_prefix) - +{ cd $($(1)_build_dir); $($(1)_stage_env) $($(1)_stage_cmds); } $$($(1)_logging) + +{ cd $($(1)_build_dir); export $($(1)_stage_env); $($(1)_stage_cmds); } $$($(1)_logging) rm -rf $($(1)_extract_dir) touch $$@ $($(1)_postprocessed): | $($(1)_staged) diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index ffb70e0d93ca6..1520280637f77 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -34,6 +34,7 @@ $(package)_extra_sources = $($(package)_qttranslations_file_name) $(package)_extra_sources += $($(package)_qttools_file_name) define $(package)_set_vars +$(package)_config_env = QT_MAC_SDK_NO_VERSION_CHECK=1 $(package)_config_opts_release = -release $(package)_config_opts_release += -silent $(package)_config_opts_debug = -debug @@ -273,9 +274,6 @@ define $(package)_preprocess_cmds endef define $(package)_config_cmds - export PKG_CONFIG_SYSROOT_DIR=/ && \ - export PKG_CONFIG_LIBDIR=$(host_prefix)/lib/pkgconfig && \ - export QT_MAC_SDK_NO_VERSION_CHECK=1 && \ cd qtbase && \ ./configure -top-level $($(package)_config_opts) endef From 662302c42bbf3c9d5a5fb4a81ef9591f113c9e6c Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 6 Jan 2023 16:31:26 +0100 Subject: [PATCH 07/21] Merge bitcoin/bitcoin#26805: tests: Use unique port for ZMQ tests to allow for multiple test instances c6119f478876f245ca5c65dd05da4cdc5be0e91f tests: Use unique port for ZMQ tests (Andrew Chow) Pull request description: The ZMQ interface tests should use unique ports as we do for the p2p and rpc ports so that multiple instances of the test can be run at the same time. Without this, the test may hang until killed, or fail. ACKs for top commit: MarcoFalke: ACK c6119f478876f245ca5c65dd05da4cdc5be0e91f Tree-SHA512: 2ca3ed2f35e5a83d7ab83740674fed362a8d146dc751156cfe100133a591347cd1ac9d164046f1744d65451a57c52cb22d3bb2161105f421f8f655c4a2512c59 --- test/functional/interface_zmq.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index d8565814c2bcb..e25067455b728 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -16,6 +16,7 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, + p2p_port, ) from test_framework.netutil import test_ipv6_local from time import sleep @@ -102,6 +103,7 @@ def set_test_params(self): # This test isn't testing txn relay/timing, so set whitelist on the # peers for instant txn relay. This speeds up the test run time 2-3x. self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes + self.zmq_port_base = p2p_port(self.num_nodes + 1) def skip_test_if_missing_module(self): self.skip_if_no_py3_zmq() @@ -177,7 +179,7 @@ def test_basic(self): self.restart_node(0, ["-zmqpubrawtx=foo", "-zmqpubhashtx=bar"]) self.zmq_context = zmq.Context() - address = 'tcp://127.0.0.1:28332' + address = f"tcp://127.0.0.1:{self.zmq_port_base}" subs = self.setup_zmq_test([(topic, address) for topic in ["hashblock", "hashtx", "rawblock", "rawtx"]]) hashblock = subs[0] @@ -248,7 +250,7 @@ def test_reorg(self): self.log.info("Skipping reorg test because wallet is disabled") return - address = 'tcp://127.0.0.1:28333' + address = f"tcp://127.0.0.1:{self.zmq_port_base}" # Should only notify the tip if a reorg occurs hashblock, hashtx = self.setup_zmq_test( @@ -302,7 +304,7 @@ def test_sequence(self): <32-byte hash>A<8-byte LE uint> : Transactionhash added mempool """ self.log.info("Testing 'sequence' publisher") - [seq] = self.setup_zmq_test([("sequence", "tcp://127.0.0.1:28333")]) + [seq] = self.setup_zmq_test([("sequence", f"tcp://127.0.0.1:{self.zmq_port_base}")]) self.disconnect_nodes(0, 1) # Mempool sequence number starts at 1 @@ -435,7 +437,7 @@ def test_mempool_sync(self): return self.log.info("Testing 'mempool sync' usage of sequence notifier") - [seq] = self.setup_zmq_test([("sequence", "tcp://127.0.0.1:28333")]) + [seq] = self.setup_zmq_test([("sequence", f"tcp://127.0.0.1:{self.zmq_port_base}")]) # In-memory counter, should always start at 1 next_mempool_seq = self.nodes[0].getrawmempool(mempool_sequence=True)["mempool_sequence"] @@ -537,8 +539,8 @@ def test_multiple_interfaces(self): # chain lengths on node0 and node1; for this test we only need node0, so # we can disable syncing blocks on the setup) subscribers = self.setup_zmq_test([ - ("hashblock", "tcp://127.0.0.1:28334"), - ("hashblock", "tcp://127.0.0.1:28335"), + ("hashblock", f"tcp://127.0.0.1:{self.zmq_port_base + 1}"), + ("hashblock", f"tcp://127.0.0.1:{self.zmq_port_base + 2}"), ], sync_blocks=False) # Generate 1 block in nodes[0] and receive all notifications @@ -555,7 +557,7 @@ def test_ipv6(self): self.log.info("Testing IPv6") # Set up subscriber using IPv6 loopback address subscribers = self.setup_zmq_test([ - ("hashblock", "tcp://[::1]:28332") + ("hashblock", f"tcp://[::1]:{self.zmq_port_base}") ], ipv6=True) # Generate 1 block in nodes[0] From 8cc5f11a2f4f0e0862c98dd223e22ea1c866074b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Jan 2023 13:12:02 +0100 Subject: [PATCH 08/21] Merge bitcoin/bitcoin#26506: refactor: rpc: use convenience fn to auto parse non-string parameters 6d0ab07e817725369df699791b9c5b2fae204990 refactor: use convenience fn to auto parse non-string parameters (stickies-v) Pull request description: Minimizes code duplication and improves function naming by having a single (overloaded) convenience function `ParseIfNonString` that both checks if the parameter is a non-string parameter and automatically parses the value if so. ACKs for top commit: aureleoules: ACK 6d0ab07e817725369df699791b9c5b2fae204990 Tree-SHA512: 8cbf68a17cfbdce1e31a19916f447a2965c139fdef00c19e32a9b679f4a4015dfe69ceea0bbe1723711e1c5033ea8d4005d1f4485dfbeea22226140f8cbe8aa3 --- src/rpc/client.cpp | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index c97421afd2562..fce46253db364 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -250,11 +250,16 @@ class CRPCConvertTable public: CRPCConvertTable(); - bool convert(const std::string& method, int idx) { - return (members.count(std::make_pair(method, idx)) > 0); + /** Return arg_value as UniValue, and first parse it if it is a non-string parameter */ + UniValue ArgToUniValue(const std::string& arg_value, const std::string& method, int param_idx) + { + return members.count(std::make_pair(method, param_idx)) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value; } - bool convert(const std::string& method, const std::string& name) { - return (membersByName.count(std::make_pair(method, name)) > 0); + + /** Return arg_value as UniValue, and first parse it if it is a non-string parameter */ + UniValue ArgToUniValue(const std::string& arg_value, const std::string& method, const std::string& param_name) + { + return membersByName.count(std::make_pair(method, param_name)) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value; } }; @@ -286,14 +291,7 @@ UniValue RPCConvertValues(const std::string &strMethod, const std::vector Date: Wed, 18 Jan 2023 15:37:51 +0000 Subject: [PATCH 09/21] Merge bitcoin/bitcoin#26873: doc: add databases/py-sqlite3 to FreeBSD test suite deps 376e01b382679b49d5c8464c869d14eca23ab4b9 doc: add databases/py-sqlite3 to FreeBSD test suite deps (fanquake) Pull request description: Adds missing documentation. See also https://cirrus-ci.com/task/5639240319500288. ACKs for top commit: john-moffett: ACK 376e01b382679b49d5c8464c869d14eca23ab4b9 Tree-SHA512: a7b8d0bae00c3538934d23eae207b7927a64e748eb228ac6c5754aee0e9b6796c0f39066c7d81cc009bf442c8b1a0c31739adde87d1ebc3445aed54dda47c9ce --- doc/build-freebsd.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/build-freebsd.md b/doc/build-freebsd.md index d5a3004c8aa0b..dd734f7787d15 100644 --- a/doc/build-freebsd.md +++ b/doc/build-freebsd.md @@ -96,7 +96,7 @@ There is an included test suite that is useful for testing code changes when dev To run the test suite (recommended), you will need to have Python 3 installed: ```bash -pkg install python3 +pkg install python3 databases/py-sqlite3 ``` --- From d1b738637407fac9eca2448b71c3b0c6fc43adee Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 24 Jan 2023 12:53:00 +0100 Subject: [PATCH 10/21] Merge bitcoin/bitcoin#26930: fuzz: Actually use mocked mempool in tx_pool target 9ab62d71fb1a54430ff5071bdb1120a414061288 [fuzz] Actually use mocked mempool in tx_pool target (dergoegge) Pull request description: The current tx_pool target uses the default mempool, making the target non-deterministic. This PR replaces the active chainstate's mempool (i.e. the node's default mempool) with the already present mocked mempool in the target. ACKs for top commit: fanquake: ACK 9ab62d71fb1a54430ff5071bdb1120a414061288 Tree-SHA512: fe9af3dbdd13cb569fdc2ddbb4290b5ce94206ae83d94267c6365ed0ee9bbe072fcfe7fd632a1a8522dce44608e89aba2f398c1e20bd250484bbadb78143320c --- src/test/fuzz/tx_pool.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index ec5b6b3541fd8..5b8888fd077de 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -297,7 +297,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const auto& node = g_setup->m_node; - auto& chainstate = node.chainman->ActiveChainstate(); + auto& chainstate{static_cast(node.chainman->ActiveChainstate())}; MockTime(fuzzed_data_provider, chainstate); SetMempoolConstraints(*node.args, fuzzed_data_provider); @@ -315,6 +315,8 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) CTxMemPool tx_pool_{/* estimator */ nullptr, /* check_ratio */ 1}; MockedTxPool& tx_pool = *static_cast(&tx_pool_); + chainstate.SetMempool(&tx_pool); + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) { const auto mut_tx = ConsumeTransaction(fuzzed_data_provider, txids); From c681aaad3013f855229a798f1c6a5d11b1acf756 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 28 Jan 2023 15:33:14 +0000 Subject: [PATCH 11/21] Merge bitcoin/bitcoin#22811: build: Fix depends build system when working with subtargets 978852aad8e295c26801e17f2ee9e58edd4a5703 build: Fix depends build system when working with subtargets (Hennadii Stepanov) Pull request description: On master (f3e0ace8ecd84009a23da6b0de47f01d79c45772), the depends build system does _not_ guarantee that dependencies packages are available for `$(package)_built` target because these dependencies being prepared in `$(host_prefix)` at `$(package)_configured` target can be wiped out during building other package. Please consider: ``` $ cd depends $ make clean $ make fontconfig_configured $ make ... CC fcdir.lo In file included from fcftint.h:26, from fcdir.c:26: ../fontconfig/fcfreetype.h:27:10: fatal error: ft2build.h: No such file or directory 27 | #include | ^~~~~~~~~~~~ compilation terminated. make[4]: *** [Makefile:642: fcdir.lo] Error 1 make[4]: *** Waiting for unfinished jobs.... make[4]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94/src' make[3]: *** [Makefile:503: all] Error 2 make[3]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94/src' make[2]: *** [Makefile:581: all-recursive] Error 1 make[2]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94' make[1]: *** [Makefile:465: all] Error 2 make[1]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94' make: *** [funcs.mk:288: /home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94/./.stamp_built] Error 2 ``` The following commands: ``` $ cd depends $ make clean $ make qt_configured $ make ``` also fail. The similar issue was reported earlier: #21381. This PR guarantees that dependencies packages are available for `$(package)_built` target. Guix builds: ``` accef9ffccfe280fec1114c0440092ed5d792e9c53d95abc7fe65435aa136656 guix-build-978852aad8e2/output/aarch64-linux-gnu/SHA256SUMS.part a75f1250975525a21d2e213e23f1f0dab516d2b28d0c7d747de292fe5c906013 guix-build-978852aad8e2/output/aarch64-linux-gnu/bitcoin-978852aad8e2-aarch64-linux-gnu-debug.tar.gz d20787af2e7a14a3b7b1d21e0d8784aa6ebad1e916f02aebfa25afe9229ba43c guix-build-978852aad8e2/output/aarch64-linux-gnu/bitcoin-978852aad8e2-aarch64-linux-gnu.tar.gz 11c94a39c084763858c6de31b221868e52554f5500c0dc5589def429bb6b54c8 guix-build-978852aad8e2/output/arm-linux-gnueabihf/SHA256SUMS.part 3935b0e14d78800977dc813a3824215797096b6fb29c84b5996f48338908a65f guix-build-978852aad8e2/output/arm-linux-gnueabihf/bitcoin-978852aad8e2-arm-linux-gnueabihf-debug.tar.gz c828c3f87a453db443a385a266331661f623f8f4684d88eb006290c83bfa8150 guix-build-978852aad8e2/output/arm-linux-gnueabihf/bitcoin-978852aad8e2-arm-linux-gnueabihf.tar.gz b6ff14e1cc36e568fc726b50300f77498560322b3582738eb70e7144784f7e63 guix-build-978852aad8e2/output/arm64-apple-darwin/SHA256SUMS.part 2a3e6ba5843eaf9562e9dcfdb4595024a71738079cea00e391558feca4d5bde1 guix-build-978852aad8e2/output/arm64-apple-darwin/bitcoin-978852aad8e2-arm64-apple-darwin-unsigned.dmg ffd2ad39e8b9f95dd714513ba1e1c77666b0f3cc4b67be4ab763ebd431fe9276 guix-build-978852aad8e2/output/arm64-apple-darwin/bitcoin-978852aad8e2-arm64-apple-darwin-unsigned.tar.gz 5c9f8acd1777effc1e860b64143ba9d06ba5e3d0330e7341529eeae5cc6b3c5e guix-build-978852aad8e2/output/arm64-apple-darwin/bitcoin-978852aad8e2-arm64-apple-darwin.tar.gz e34c693ecef6159c57fdedabff9dc3d69ec20387966083b828532c58e1e6e30b guix-build-978852aad8e2/output/dist-archive/bitcoin-978852aad8e2.tar.gz 8de4681114d96425bf9b0ccc47d49f696293ead514faa3fa83ddcccfdca2eeb2 guix-build-978852aad8e2/output/powerpc64-linux-gnu/SHA256SUMS.part d780330a105931eb4c66a536c332eb09e4b6d8c288832bb5a74b931c4600c0b3 guix-build-978852aad8e2/output/powerpc64-linux-gnu/bitcoin-978852aad8e2-powerpc64-linux-gnu-debug.tar.gz af036a6d714ef362a2facfe4e5c63deaa9dfa391d20542ead9ffb4a43b2b7ca2 guix-build-978852aad8e2/output/powerpc64-linux-gnu/bitcoin-978852aad8e2-powerpc64-linux-gnu.tar.gz dc6b4365632e7de386ead512b1cdcdd3c985623f63cff8b62974bd9ed8c05d5d guix-build-978852aad8e2/output/powerpc64le-linux-gnu/SHA256SUMS.part 5543a6dec58515186b71139a4a5c603d85638672f205e7c9fabb281c98018461 guix-build-978852aad8e2/output/powerpc64le-linux-gnu/bitcoin-978852aad8e2-powerpc64le-linux-gnu-debug.tar.gz dd791be2e61ce6cbd3e14c165ce2f8c2d22881992e0df72bd338423d092cc467 guix-build-978852aad8e2/output/powerpc64le-linux-gnu/bitcoin-978852aad8e2-powerpc64le-linux-gnu.tar.gz 2b740db8e5b9c435be3e7b186c3b4a40885302243326ec990e24fe4ba4f777da guix-build-978852aad8e2/output/riscv64-linux-gnu/SHA256SUMS.part de899c89874d4f34afeca179a6c7c8f49b0a975983ab2b31afd9f2d365674578 guix-build-978852aad8e2/output/riscv64-linux-gnu/bitcoin-978852aad8e2-riscv64-linux-gnu-debug.tar.gz 9052b1db22c56692d99a61c3783b36c6f76537d9aec14f17d87a155beac82532 guix-build-978852aad8e2/output/riscv64-linux-gnu/bitcoin-978852aad8e2-riscv64-linux-gnu.tar.gz 5e79ddf57a94c5978ad819896786107f735d5742bbd042c2c64ae2d0681ce53a guix-build-978852aad8e2/output/x86_64-apple-darwin/SHA256SUMS.part 96443ad839f87c723db1c0a96d8ead0afc69e9d96ad45b5814344866da2dae73 guix-build-978852aad8e2/output/x86_64-apple-darwin/bitcoin-978852aad8e2-x86_64-apple-darwin-unsigned.dmg 14b0a3081772e81a463398a2702aca039d2f276e301dee9f5a0ccffbb09e2749 guix-build-978852aad8e2/output/x86_64-apple-darwin/bitcoin-978852aad8e2-x86_64-apple-darwin-unsigned.tar.gz 6bfb8252524202028308267f5e96bc30f284052f5feaa58ed3697dde27a3130f guix-build-978852aad8e2/output/x86_64-apple-darwin/bitcoin-978852aad8e2-x86_64-apple-darwin.tar.gz 5f8ea6297e246b08ffd806913897cc863feeec6522fcfb4456a59c5f154e0c2d guix-build-978852aad8e2/output/x86_64-linux-gnu/SHA256SUMS.part 40d1bcf491660d54fe20b2db24828ebf61be848eefdc38fba09ed43f6bdba4b1 guix-build-978852aad8e2/output/x86_64-linux-gnu/bitcoin-978852aad8e2-x86_64-linux-gnu-debug.tar.gz 3200e67a4dea115e8e341b4d71d84dc5e8bd2ae35e550cde6aef88d120c65eae guix-build-978852aad8e2/output/x86_64-linux-gnu/bitcoin-978852aad8e2-x86_64-linux-gnu.tar.gz 0b0bf7effc493ecc68398f23fc81647f64fdee115e8ccd7ae91e7881804ec328 guix-build-978852aad8e2/output/x86_64-w64-mingw32/SHA256SUMS.part e2064c9ddeb4af18468f37ba8cf70004062c31e1387b4cc0fe4b445fae518e8d guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64-debug.zip be347a901b896e0a1dc2f0f5a7f84614075805cccf1f2af8ec8df678d086fdbc guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64-setup-unsigned.exe bab8700e9e266970e8c7cad494902058ad12d1f2a6462e0039daa637b1a0ce0d guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64-unsigned.tar.gz c8e55e64b248fd7c9056fe811a1eba992bbb92e44857204e3024416d9ba6307d guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64.zip ``` ACKs for top commit: TheCharlatan: tACK 978852aad8e295c26801e17f2ee9e58edd4a5703 Tree-SHA512: c195484274433039e327d44b1949afa296e09e7470a2b138b7a8476c8bf9c1302bc21284cd5436f09aa97824aae9f362b7932ff2937b78f79df0b43e50f3dfaa --- depends/funcs.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/depends/funcs.mk b/depends/funcs.mk index 7bc38b09970ec..6e12aae2ac203 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -76,7 +76,7 @@ $(1)_extracted=$$($(1)_extract_dir)/.stamp_extracted $(1)_preprocessed=$$($(1)_extract_dir)/.stamp_preprocessed $(1)_cleaned=$$($(1)_extract_dir)/.stamp_cleaned $(1)_built=$$($(1)_build_dir)/.stamp_built -$(1)_configured=$$($(1)_build_dir)/.stamp_configured +$(1)_configured=$(host_prefix)/.$(1)_stamp_configured $(1)_staged=$$($(1)_staging_dir)/.stamp_staged $(1)_postprocessed=$$($(1)_staging_prefix_dir)/.stamp_postprocessed $(1)_download_path_fixed=$(subst :,\:,$$($(1)_download_path)) @@ -221,8 +221,8 @@ $($(1)_preprocessed): | $($(1)_extracted) $($(1)_configured): | $($(1)_dependencies) $($(1)_preprocessed) echo Configuring $(1)... rm -rf $(host_prefix); mkdir -p $(host_prefix)/lib; cd $(host_prefix); $(foreach package,$($(1)_all_dependencies), $(build_TAR) --no-same-owner -xf $($(package)_cached); ) - mkdir -p $$(@D) - +{ cd $$(@D); export $($(1)_config_env); $($(1)_config_cmds); } $$($(1)_logging) + mkdir -p $$($(1)_build_dir) + +{ cd $$($(1)_build_dir); export $($(1)_config_env); $($(1)_config_cmds); } $$($(1)_logging) touch $$@ $($(1)_built): | $($(1)_configured) echo Building $(1)... From 2ab1989a3927209079bba0fdbe23f7588757d7a7 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 1 Feb 2023 15:56:23 +0100 Subject: [PATCH 12/21] Merge bitcoin/bitcoin#27010: refactor: use `Hash` helpers for double-SHA256 calculations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd refactor: use `Hash` helper for double-SHA256 calculations (Sebastian Falbesoner) Pull request description: We have two helper templates `Hash(const T& in1)` and `Hash(const T& in1, const T& in2)` available for calculating the double-SHA256 hash of one object or two concatenated objects, respectively: https://github.com/bitcoin/bitcoin/blob/b5868f4b1f884e8d6612f34ca4005fe3a992053d/src/hash.h#L74-L89 This PR uses them in order to increase readability and simplify the code. As in #15294 (which inspired this PR, doing the same for RIPEMD160), the helper is not utilized in validation.cpp and script/interpreter.cpp to avoid touching consensus-relevant code. ACKs for top commit: john-moffett: ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd stickies-v: ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd MarcoFalke: review ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd 😬 Tree-SHA512: 11d7e3d00c89685107784010fbffb33ccafb4d1b6a76c4dceb937b29bb234ef4d54581b16bd0737c8d2994a90cf4fe10a9738c7cc5b6d085c6a819f06176dab9 --- src/blockfilter.cpp | 15 ++------------- src/index/blockfilterindex.cpp | 4 +--- src/key.cpp | 3 +-- src/test/key_tests.cpp | 3 +-- src/test/merkle_tests.cpp | 6 +++--- 5 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index e7cc29afda705..cc15fe9e5203c 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -259,21 +259,10 @@ bool BlockFilter::BuildParams(GCSFilter::Params& params) const uint256 BlockFilter::GetHash() const { - const std::vector& data = GetEncodedFilter(); - - uint256 result; - CHash256().Write(data).Finalize(result); - return result; + return Hash(GetEncodedFilter()); } uint256 BlockFilter::ComputeHeader(const uint256& prev_header) const { - const uint256& filter_hash = GetHash(); - - uint256 result; - CHash256() - .Write(filter_hash) - .Write(prev_header) - .Finalize(result); - return result; + return Hash(GetHash(), prev_header); } diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 2e84934eba916..8dd9b0094235f 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -159,9 +159,7 @@ bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& std::vector encoded_filter; try { filein >> block_hash >> encoded_filter; - uint256 result; - CHash256().Write(encoded_filter).Finalize(result); - if (result != hash) return error("Checksum mismatch in filter decode."); + if (Hash(encoded_filter) != hash) return error("Checksum mismatch in filter decode."); filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter), /*skip_decode_check=*/true); } catch (const std::exception& e) { diff --git a/src/key.cpp b/src/key.cpp index 066dcd783aa15..66209c7274b4a 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -243,8 +243,7 @@ bool CKey::VerifyPubKey(const CPubKey& pubkey) const { unsigned char rnd[8]; std::string str = "Bitcoin key verification\n"; GetRandBytes(rnd); - uint256 hash; - CHash256().Write(MakeUCharSpan(str)).Write(rnd).Finalize(hash); + uint256 hash{Hash(str, rnd)}; std::vector vchSig; Sign(hash, vchSig); return pubkey.Verify(hash, vchSig); diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 9058cdbaaf2a8..31e58962d7b8f 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -205,8 +205,7 @@ BOOST_AUTO_TEST_CASE(key_key_negation) unsigned char rnd[8]; std::string str = "Bitcoin key verification\n"; GetRandBytes(rnd); - uint256 hash; - CHash256().Write(MakeUCharSpan(str)).Write(rnd).Finalize(hash); + uint256 hash{Hash(str, rnd)}; // import the static test key CKey key = DecodeSecret(strSecret1C); diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 7b08b1da5d5fb..cb0549fbafe75 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -60,7 +60,7 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot } } mutated |= (inner[level] == h); - CHash256().Write(inner[level]).Write(h).Finalize(h); + h = Hash(inner[level], h); } // Store the resulting hash at inner position level. inner[level] = h; @@ -86,7 +86,7 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot if (pbranch && matchh) { pbranch->push_back(h); } - CHash256().Write(h).Write(h).Finalize(h); + h = Hash(h, h); // Increment count to the value it would have if two entries at this // level had existed. count += (((uint32_t)1) << level); @@ -101,7 +101,7 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot matchh = true; } } - CHash256().Write(inner[level]).Write(h).Finalize(h); + h = Hash(inner[level], h); level++; } } From 44e6c9e9025363cd142b9f28c918232642ce88ad Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 2 Feb 2023 16:51:35 +0000 Subject: [PATCH 13/21] Merge bitcoin/bitcoin#27004: test: Use std::unique_ptr over manual delete in coins_tests fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d test: Use std::unique_ptr over manual delete in coins_tests (MarcoFalke) Pull request description: Makes the code smaller and easier to read ACKs for top commit: stickies-v: ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d john-moffett: ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d Tree-SHA512: 30d2d2097906e61fdef47a52fc6a0c5ce2417bc41c3c82dafc1b216c655f31dabf9c1c13759575a696f61bbdfdba3f442be032d5e5145b7a54fae2a927824621 --- src/test/coins_tests.cpp | 54 +++++++++++++--------------------------- 1 file changed, 17 insertions(+), 37 deletions(-) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 75b25143e2884..6c8ab69bb5364 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -132,8 +132,8 @@ void SimulationTest(CCoinsView* base, bool fake_best_block) std::map result; // The cache stack. - std::vector stack; // A stack of CCoinsViewCaches on top. - stack.push_back(new CCoinsViewCacheTest(base)); // Start with one cache. + std::vector> stack; // A stack of CCoinsViewCaches on top. + stack.push_back(std::make_unique(base)); // Start with one cache. // Use a limited set of random transaction ids, so we do test overwriting entries. std::vector txids; @@ -219,7 +219,7 @@ void SimulationTest(CCoinsView* base, bool fake_best_block) found_an_entry = true; } } - for (const CCoinsViewCacheTest *test : stack) { + for (const auto& test : stack) { test->SelfTest(); } } @@ -242,18 +242,17 @@ void SimulationTest(CCoinsView* base, bool fake_best_block) bool should_erase = InsecureRandRange(4) < 3; BOOST_CHECK(should_erase ? stack.back()->Flush() : stack.back()->Sync()); flushed_without_erase |= !should_erase; - delete stack.back(); stack.pop_back(); } if (stack.size() == 0 || (stack.size() < 4 && InsecureRandBool())) { //Add a new cache CCoinsView* tip = base; if (stack.size() > 0) { - tip = stack.back(); + tip = stack.back().get(); } else { removed_all_caches = true; } - stack.push_back(new CCoinsViewCacheTest(tip)); + stack.push_back(std::make_unique(tip)); if (stack.size() == 4) { reached_4_caches = true; } @@ -261,12 +260,6 @@ void SimulationTest(CCoinsView* base, bool fake_best_block) } } - // Clean up the stack. - while (stack.size() > 0) { - delete stack.back(); - stack.pop_back(); - } - // Verify coverage. BOOST_CHECK(removed_all_caches); BOOST_CHECK(reached_4_caches); @@ -322,8 +315,8 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // The cache stack. CCoinsViewTest base; // A CCoinsViewTest at the bottom. - std::vector stack; // A stack of CCoinsViewCaches on top. - stack.push_back(new CCoinsViewCacheTest(&base)); // Start with one cache. + std::vector> stack; // A stack of CCoinsViewCaches on top. + stack.push_back(std::make_unique(&base)); // Start with one cache. // Track the txids we've used in various sets std::set coinbase_coins; @@ -488,25 +481,18 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) // Every 100 iterations, change the cache stack. if (stack.size() > 0 && InsecureRandBool() == 0) { BOOST_CHECK(stack.back()->Flush()); - delete stack.back(); stack.pop_back(); } if (stack.size() == 0 || (stack.size() < 4 && InsecureRandBool())) { CCoinsView* tip = &base; if (stack.size() > 0) { - tip = stack.back(); + tip = stack.back().get(); } - stack.push_back(new CCoinsViewCacheTest(tip)); + stack.push_back(std::make_unique(tip)); } } } - // Clean up the stack. - while (stack.size() > 0) { - delete stack.back(); - stack.pop_back(); - } - // Verify coverage. BOOST_CHECK(spent_a_duplicate_coinbase); @@ -920,7 +906,7 @@ Coin MakeCoin() void TestFlushBehavior( CCoinsViewCacheTest* view, CCoinsViewDB& base, - std::vector& all_caches, + std::vector>& all_caches, bool do_erasing_flush) { CAmount value; @@ -931,7 +917,7 @@ void TestFlushBehavior( auto flush_all = [&all_caches](bool erase) { // Flush in reverse order to ensure that flushes happen from children up. for (auto i = all_caches.rbegin(); i != all_caches.rend(); ++i) { - auto cache = *i; + auto& cache = *i; // hashBlock must be filled before flushing to disk; value is // unimportant here. This is normally done during connect/disconnect block. cache->SetBestBlock(InsecureRand256()); @@ -1087,19 +1073,13 @@ BOOST_AUTO_TEST_CASE(ccoins_flush_behavior) { // Create two in-memory caches atop a leveldb view. CCoinsViewDB base{"test", /*nCacheSize=*/ 1 << 23, /*fMemory=*/ true, /*fWipe=*/ false}; - std::vector caches; - caches.push_back(new CCoinsViewCacheTest(&base)); - caches.push_back(new CCoinsViewCacheTest(caches.back())); - - for (CCoinsViewCacheTest* view : caches) { - TestFlushBehavior(view, base, caches, /*do_erasing_flush=*/ false); - TestFlushBehavior(view, base, caches, /*do_erasing_flush=*/ true); - } + std::vector> caches; + caches.push_back(std::make_unique(&base)); + caches.push_back(std::make_unique(caches.back().get())); - // Clean up the caches. - while (caches.size() > 0) { - delete caches.back(); - caches.pop_back(); + for (const auto& view : caches) { + TestFlushBehavior(view.get(), base, caches, /*do_erasing_flush=*/false); + TestFlushBehavior(view.get(), base, caches, /*do_erasing_flush=*/true); } } From a7e3c2c916cb493dedeeaa2a908a83cd69422ca4 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 3 Feb 2023 18:57:14 +0000 Subject: [PATCH 14/21] Merge bitcoin-core/gui#705: doc: Fix comment about how wallet txs are sorted c497a198db6f417d2612078a9fbc101e259fab33 Fix comment about how wallet txs are sorted (John Moffett) Pull request description: The wallet transactions in the node are not sorted by txid (or any hash) since https://github.com/bitcoin/bitcoin/pull/24699. This is how they're stored in memory now: https://github.com/bitcoin-core/gui/blob/835212cd1d8f8fc7f19775f5ff8cc21c099122b2/src/wallet/wallet.h#L397-L399 ACKs for top commit: achow101: ACK c497a198db6f417d2612078a9fbc101e259fab33 jarolrod: ACK c497a198db6f417d2612078a9fbc101e259fab33 Tree-SHA512: e72559991688452ef254474d4235dc75fac655bce04909c3a0eece907360f4c6f57707db9b4373a4bd2271b23c57e863684c33e0728adf48e477c5499cdfdad7 --- src/qt/transactiontablemodel.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 0a7b0066c38a6..ed5aa41aeb3cb 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -91,10 +91,7 @@ class TransactionTablePriv TransactionTableModel *parent; - /* Local cache of wallet. - * As it is in the same order as the CWallet, by definition - * this is sorted by sha256. - */ + //! Local cache of wallet sorted by transaction hash QList cachedWallet; /** True when model finishes loading all wallet transactions on start */ From c4760bb32e0ab2697eaa995a1ff25a5a51c1fe4c Mon Sep 17 00:00:00 2001 From: fanquake Date: Sun, 5 Feb 2023 13:44:39 +0000 Subject: [PATCH 15/21] Merge bitcoin/bitcoin#27030: Update nanobench to version v4.3.10 82f895d7b540ae421f80305a4f7cbb42905fb2c6 Update nanobench to version v4.3.10 (Martin Leitner-Ankerl) Pull request description: Nothing has changed that would affect Bitcoin's usage of nanobench. Here is a detailed list of the changes * Plenty of clang-tidy updates * documentation updates * faster Rng::shuffle * Enable perf counters on older kernels * Raise default minimum epoch time to 1ms (doesn't effect bitcoin's usage) * Add support for custom information per benchmark ACKs for top commit: hebasto: ACK 82f895d7b540ae421f80305a4f7cbb42905fb2c6, I've reviewed the code, all related changes from #26642 have been implemented. Tree-SHA512: 942518398809a2794617a347ab8182b784a8e822e84de5af078b2531eabb438412d687cac22a21936585e60e07138a89b41c28c9750744c05a3d1053f55cad01 --- src/bench/nanobench.h | 461 ++++++++++++++++++++++++++---------------- 1 file changed, 283 insertions(+), 178 deletions(-) diff --git a/src/bench/nanobench.h b/src/bench/nanobench.h index 27df08fb69087..c518b91b6a1b0 100644 --- a/src/bench/nanobench.h +++ b/src/bench/nanobench.h @@ -7,7 +7,7 @@ // // Licensed under the MIT License . // SPDX-License-Identifier: MIT -// Copyright (c) 2019-2021 Martin Ankerl +// Copyright (c) 2019-2023 Martin Leitner-Ankerl // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -31,19 +31,20 @@ #define ANKERL_NANOBENCH_H_INCLUDED // see https://semver.org/ -#define ANKERL_NANOBENCH_VERSION_MAJOR 4 // incompatible API changes -#define ANKERL_NANOBENCH_VERSION_MINOR 3 // backwards-compatible changes -#define ANKERL_NANOBENCH_VERSION_PATCH 6 // backwards-compatible bug fixes +#define ANKERL_NANOBENCH_VERSION_MAJOR 4 // incompatible API changes +#define ANKERL_NANOBENCH_VERSION_MINOR 3 // backwards-compatible changes +#define ANKERL_NANOBENCH_VERSION_PATCH 10 // backwards-compatible bug fixes /////////////////////////////////////////////////////////////////////////////////////////////////// // public facing api - as minimal as possible /////////////////////////////////////////////////////////////////////////////////////////////////// -#include // high_resolution_clock -#include // memcpy -#include // for std::ostream* custom output target in Config -#include // all names -#include // holds all results +#include // high_resolution_clock +#include // memcpy +#include // for std::ostream* custom output target in Config +#include // all names +#include // holds context information of results +#include // holds all results #define ANKERL_NANOBENCH(x) ANKERL_NANOBENCH_PRIVATE_##x() @@ -91,7 +92,7 @@ #define ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS() 0 #if defined(__linux__) && !defined(ANKERL_NANOBENCH_DISABLE_PERF_COUNTERS) # include -# if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 14, 0) +# if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 3, 0) // PERF_COUNT_HW_REF_CPU_CYCLES only available since kernel 3.3 // PERF_FLAG_FD_CLOEXEC since kernel 3.14 # undef ANKERL_NANOBENCH_PRIVATE_PERF_COUNTERS @@ -144,43 +145,45 @@ class BigO; * * `{{#result}}` Marks the begin of the result layer. Whatever comes after this will be instantiated as often as * a benchmark result is available. Within it, you can use these tags: * - * * `{{title}}` See Bench::title(). + * * `{{title}}` See Bench::title. * - * * `{{name}}` Benchmark name, usually directly provided with Bench::run(), but can also be set with Bench::name(). + * * `{{name}}` Benchmark name, usually directly provided with Bench::run, but can also be set with Bench::name. * - * * `{{unit}}` Unit, e.g. `byte`. Defaults to `op`, see Bench::title(). + * * `{{unit}}` Unit, e.g. `byte`. Defaults to `op`, see Bench::unit. * - * * `{{batch}}` Batch size, see Bench::batch(). + * * `{{batch}}` Batch size, see Bench::batch. * - * * `{{complexityN}}` Value used for asymptotic complexity calculation. See Bench::complexityN(). + * * `{{complexityN}}` Value used for asymptotic complexity calculation. See Bench::complexityN. * - * * `{{epochs}}` Number of epochs, see Bench::epochs(). + * * `{{epochs}}` Number of epochs, see Bench::epochs. * * * `{{clockResolution}}` Accuracy of the clock, i.e. what's the smallest time possible to measure with the clock. * For modern systems, this can be around 20 ns. This value is automatically determined by nanobench at the first * benchmark that is run, and used as a static variable throughout the application's runtime. * - * * `{{clockResolutionMultiple}}` Configuration multiplier for `clockResolution`. See Bench::clockResolutionMultiple(). + * * `{{clockResolutionMultiple}}` Configuration multiplier for `clockResolution`. See Bench::clockResolutionMultiple. * This is the target runtime for each measurement (epoch). That means the more accurate your clock is, the faster * will be the benchmark. Basing the measurement's runtime on the clock resolution is the main reason why nanobench is so fast. * * * `{{maxEpochTime}}` Configuration for a maximum time each measurement (epoch) is allowed to take. Note that at least - * a single iteration will be performed, even when that takes longer than maxEpochTime. See Bench::maxEpochTime(). + * a single iteration will be performed, even when that takes longer than maxEpochTime. See Bench::maxEpochTime. * - * * `{{minEpochTime}}` Minimum epoch time, usually not set. See Bench::minEpochTime(). + * * `{{minEpochTime}}` Minimum epoch time, defaults to 1ms. See Bench::minEpochTime. * - * * `{{minEpochIterations}}` See Bench::minEpochIterations(). + * * `{{minEpochIterations}}` See Bench::minEpochIterations. * - * * `{{epochIterations}}` See Bench::epochIterations(). + * * `{{epochIterations}}` See Bench::epochIterations. * - * * `{{warmup}}` Number of iterations used before measuring starts. See Bench::warmup(). + * * `{{warmup}}` Number of iterations used before measuring starts. See Bench::warmup. * - * * `{{relative}}` True or false, depending on the setting you have used. See Bench::relative(). + * * `{{relative}}` True or false, depending on the setting you have used. See Bench::relative. + * + * * `{{context(variableName)}}` See Bench::context. * * Apart from these tags, it is also possible to use some mathematical operations on the measurement data. The operations * are of the form `{{command(name)}}`. Currently `name` can be one of `elapsed`, `iterations`. If performance counters * are available (currently only on current Linux systems), you also have `pagefaults`, `cpucycles`, - * `contextswitches`, `instructions`, `branchinstructions`, and `branchmisses`. All the measuers (except `iterations`) are + * `contextswitches`, `instructions`, `branchinstructions`, and `branchmisses`. All the measures (except `iterations`) are * provided for a single iteration (so `elapsed` is the time a single iteration took). The following tags are available: * * * `{{median()}}` Calculate median of a measurement data set, e.g. `{{median(elapsed)}}`. @@ -201,7 +204,7 @@ class BigO; * This measurement is a bit hard to interpret, but it is very robust against outliers. E.g. a value of 5% means that half of the * measurements deviate less than 5% from the median, and the other deviate more than 5% from the median. * - * * `{{sum()}}` Sums of all the measurements. E.g. `{{sum(iterations)}}` will give you the total number of iterations + * * `{{sum()}}` Sum of all the measurements. E.g. `{{sum(iterations)}}` will give you the total number of iterations * measured in this benchmark. * * * `{{minimum()}}` Minimum of all measurements. @@ -244,21 +247,21 @@ class BigO; * For the layer tags *result* and *measurement* you additionally can use these special markers: * * * ``{{#-first}}`` - Begin marker of a template that will be instantiated *only for the first* entry in the layer. Use is only - * allowed between the begin and end marker of the layer allowed. So between ``{{#result}}`` and ``{{/result}}``, or between + * allowed between the begin and end marker of the layer. So between ``{{#result}}`` and ``{{/result}}``, or between * ``{{#measurement}}`` and ``{{/measurement}}``. Finish the template with ``{{/-first}}``. * * * ``{{^-first}}`` - Begin marker of a template that will be instantiated *for each except the first* entry in the layer. This, - * this is basically the inversion of ``{{#-first}}``. Use is only allowed between the begin and end marker of the layer allowed. + * this is basically the inversion of ``{{#-first}}``. Use is only allowed between the begin and end marker of the layer. * So between ``{{#result}}`` and ``{{/result}}``, or between ``{{#measurement}}`` and ``{{/measurement}}``. * * * ``{{/-first}}`` - End marker for either ``{{#-first}}`` or ``{{^-first}}``. * * * ``{{#-last}}`` - Begin marker of a template that will be instantiated *only for the last* entry in the layer. Use is only - * allowed between the begin and end marker of the layer allowed. So between ``{{#result}}`` and ``{{/result}}``, or between + * allowed between the begin and end marker of the layer. So between ``{{#result}}`` and ``{{/result}}``, or between * ``{{#measurement}}`` and ``{{/measurement}}``. Finish the template with ``{{/-last}}``. * * * ``{{^-last}}`` - Begin marker of a template that will be instantiated *for each except the last* entry in the layer. This, - * this is basically the inversion of ``{{#-last}}``. Use is only allowed between the begin and end marker of the layer allowed. + * this is basically the inversion of ``{{#-last}}``. Use is only allowed between the begin and end marker of the layer. * So between ``{{#result}}`` and ``{{/result}}``, or between ``{{#measurement}}`` and ``{{/measurement}}``. * * * ``{{/-last}}`` - End marker for either ``{{#-last}}`` or ``{{^-last}}``. @@ -316,12 +319,12 @@ char const* csv() noexcept; See the tutorial at :ref:`tutorial-template-html` for an example. @endverbatim - @see ankerl::nanobench::render() + @see also ankerl::nanobench::render() */ char const* htmlBoxplot() noexcept; /*! - @brief Output in pyperf compatible JSON format, which can be used for more analyzations. + @brief Output in pyperf compatible JSON format, which can be used for more analyzation. @verbatim embed:rst See the tutorial at :ref:`tutorial-template-pyperf` for an example how to further analyze the output. @endverbatim @@ -378,30 +381,32 @@ struct PerfCountSet { ANKERL_NANOBENCH(IGNORE_PADDED_PUSH) struct Config { // actual benchmark config - std::string mBenchmarkTitle = "benchmark"; - std::string mBenchmarkName = "noname"; - std::string mUnit = "op"; - double mBatch = 1.0; - double mComplexityN = -1.0; - size_t mNumEpochs = 11; - size_t mClockResolutionMultiple = static_cast(1000); - std::chrono::nanoseconds mMaxEpochTime = std::chrono::milliseconds(100); - std::chrono::nanoseconds mMinEpochTime{}; - uint64_t mMinEpochIterations{1}; - uint64_t mEpochIterations{0}; // If not 0, run *exactly* these number of iterations per epoch. - uint64_t mWarmup = 0; - std::ostream* mOut = nullptr; - std::chrono::duration mTimeUnit = std::chrono::nanoseconds{1}; - std::string mTimeUnitName = "ns"; - bool mShowPerformanceCounters = true; - bool mIsRelative = false; + std::string mBenchmarkTitle = "benchmark"; // NOLINT(misc-non-private-member-variables-in-classes) + std::string mBenchmarkName = "noname"; // NOLINT(misc-non-private-member-variables-in-classes) + std::string mUnit = "op"; // NOLINT(misc-non-private-member-variables-in-classes) + double mBatch = 1.0; // NOLINT(misc-non-private-member-variables-in-classes) + double mComplexityN = -1.0; // NOLINT(misc-non-private-member-variables-in-classes) + size_t mNumEpochs = 11; // NOLINT(misc-non-private-member-variables-in-classes) + size_t mClockResolutionMultiple = static_cast(1000); // NOLINT(misc-non-private-member-variables-in-classes) + std::chrono::nanoseconds mMaxEpochTime = std::chrono::milliseconds(100); // NOLINT(misc-non-private-member-variables-in-classes) + std::chrono::nanoseconds mMinEpochTime = std::chrono::milliseconds(1); // NOLINT(misc-non-private-member-variables-in-classes) + uint64_t mMinEpochIterations{1}; // NOLINT(misc-non-private-member-variables-in-classes) + // If not 0, run *exactly* these number of iterations per epoch. + uint64_t mEpochIterations{0}; // NOLINT(misc-non-private-member-variables-in-classes) + uint64_t mWarmup = 0; // NOLINT(misc-non-private-member-variables-in-classes) + std::ostream* mOut = nullptr; // NOLINT(misc-non-private-member-variables-in-classes) + std::chrono::duration mTimeUnit = std::chrono::nanoseconds{1}; // NOLINT(misc-non-private-member-variables-in-classes) + std::string mTimeUnitName = "ns"; // NOLINT(misc-non-private-member-variables-in-classes) + bool mShowPerformanceCounters = true; // NOLINT(misc-non-private-member-variables-in-classes) + bool mIsRelative = false; // NOLINT(misc-non-private-member-variables-in-classes) + std::unordered_map mContext{}; // NOLINT(misc-non-private-member-variables-in-classes) Config(); ~Config(); - Config& operator=(Config const&); - Config& operator=(Config&&); - Config(Config const&); - Config(Config&&) noexcept; + Config& operator=(Config const& other); + Config& operator=(Config&& other) noexcept; + Config(Config const& other); + Config(Config&& other) noexcept; }; ANKERL_NANOBENCH(IGNORE_PADDED_POP) @@ -421,13 +426,13 @@ class Result { _size }; - explicit Result(Config const& benchmarkConfig); + explicit Result(Config benchmarkConfig); ~Result(); - Result& operator=(Result const&); - Result& operator=(Result&&); - Result(Result const&); - Result(Result&&) noexcept; + Result& operator=(Result const& other); + Result& operator=(Result&& other) noexcept; + Result(Result const& other); + Result(Result&& other) noexcept; // adds new measurement results // all values are scaled by iters (except iters...) @@ -442,6 +447,8 @@ class Result { ANKERL_NANOBENCH(NODISCARD) double sumProduct(Measure m1, Measure m2) const noexcept; ANKERL_NANOBENCH(NODISCARD) double minimum(Measure m) const noexcept; ANKERL_NANOBENCH(NODISCARD) double maximum(Measure m) const noexcept; + ANKERL_NANOBENCH(NODISCARD) std::string const& context(char const* variableName) const; + ANKERL_NANOBENCH(NODISCARD) std::string const& context(std::string const& variableName) const; ANKERL_NANOBENCH(NODISCARD) bool has(Measure m) const noexcept; ANKERL_NANOBENCH(NODISCARD) double get(size_t idx, Measure m) const; @@ -485,9 +492,9 @@ class Rng final { static constexpr uint64_t(max)(); /** - * As a safety precausion, we don't allow copying. Copying a PRNG would mean you would have two random generators that produce the + * As a safety precaution, we don't allow copying. Copying a PRNG would mean you would have two random generators that produce the * same sequence, which is generally not what one wants. Instead create a new rng with the default constructor Rng(), which is - * automatically seeded from `std::random_device`. If you really need a copy, use copy(). + * automatically seeded from `std::random_device`. If you really need a copy, use `copy()`. */ Rng(Rng const&) = delete; @@ -528,7 +535,7 @@ class Rng final { */ explicit Rng(uint64_t seed) noexcept; Rng(uint64_t x, uint64_t y) noexcept; - Rng(std::vector const& data); + explicit Rng(std::vector const& data); /** * Creates a copy of the Rng, thus the copy provides exactly the same random sequence as the original. @@ -620,8 +627,8 @@ class Bench { */ Bench(); - Bench(Bench&& other); - Bench& operator=(Bench&& other); + Bench(Bench&& other) noexcept; + Bench& operator=(Bench&& other) noexcept; Bench(Bench const& other); Bench& operator=(Bench const& other); ~Bench() noexcept; @@ -667,6 +674,10 @@ class Bench { */ Bench& title(char const* benchmarkTitle); Bench& title(std::string const& benchmarkTitle); + + /** + * @brief Gets the title of the benchmark + */ ANKERL_NANOBENCH(NODISCARD) std::string const& title() const noexcept; /// Name of the benchmark, will be shown in the table row. @@ -674,6 +685,31 @@ class Bench { Bench& name(std::string const& benchmarkName); ANKERL_NANOBENCH(NODISCARD) std::string const& name() const noexcept; + /** + * @brief Set context information. + * + * The information can be accessed using custom render templates via `{{context(variableName)}}`. + * Trying to render a variable that hasn't been set before raises an exception. + * Not included in (default) markdown table. + * + * @see clearContext, render + * + * @param variableName The name of the context variable. + * @param variableValue The value of the context variable. + */ + Bench& context(char const* variableName, char const* variableValue); + Bench& context(std::string const& variableName, std::string const& variableValue); + + /** + * @brief Reset context information. + * + * This may improve efficiency when using many context entries, + * or improve robustness by removing spurious context entries. + * + * @see context + */ + Bench& clearContext(); + /** * @brief Sets the batch size. * @@ -754,9 +790,9 @@ class Bench { * representation of the benchmarked code's runtime stability. * * Choose the value wisely. In practice, 11 has been shown to be a reasonable choice between runtime performance and accuracy. - * This setting goes hand in hand with minEpocIterations() (or minEpochTime()). If you are more interested in *median* runtime, you - * might want to increase epochs(). If you are more interested in *mean* runtime, you might want to increase minEpochIterations() - * instead. + * This setting goes hand in hand with minEpochIterations() (or minEpochTime()). If you are more interested in *median* runtime, + * you might want to increase epochs(). If you are more interested in *mean* runtime, you might want to increase + * minEpochIterations() instead. * * @param numEpochs Number of epochs. */ @@ -766,10 +802,10 @@ class Bench { /** * @brief Upper limit for the runtime of each epoch. * - * As a safety precausion if the clock is not very accurate, we can set an upper limit for the maximum evaluation time per + * As a safety precaution if the clock is not very accurate, we can set an upper limit for the maximum evaluation time per * epoch. Default is 100ms. At least a single evaluation of the benchmark is performed. * - * @see minEpochTime(), minEpochIterations() + * @see minEpochTime, minEpochIterations * * @param t Maximum target runtime for a single epoch. */ @@ -782,7 +818,7 @@ class Bench { * Default is zero, so we are fully relying on clockResolutionMultiple(). In most cases this is exactly what you want. If you see * that the evaluation is unreliable with a high `err%`, you can increase either minEpochTime() or minEpochIterations(). * - * @see maxEpochTime(), minEpochIterations() + * @see maxEpochTim), minEpochIterations * * @param t Minimum time each epoch should take. */ @@ -793,9 +829,9 @@ class Bench { * @brief Sets the minimum number of iterations each epoch should take. * * Default is 1, and we rely on clockResolutionMultiple(). If the `err%` is high and you want a more smooth result, you might want - * to increase the minimum number or iterations, or increase the minEpochTime(). + * to increase the minimum number of iterations, or increase the minEpochTime(). * - * @see minEpochTime(), maxEpochTime(), minEpochIterations() + * @see minEpochTime, maxEpochTime, minEpochIterations * * @param numIters Minimum number of iterations per epoch. */ @@ -886,10 +922,10 @@ class Bench { @endverbatim @tparam T Any type is cast to `double`. - @param b Length of N for the next benchmark run, so it is possible to calculate `bigO`. + @param n Length of N for the next benchmark run, so it is possible to calculate `bigO`. */ template - Bench& complexityN(T b) noexcept; + Bench& complexityN(T n) noexcept; ANKERL_NANOBENCH(NODISCARD) double complexityN() const noexcept; /*! @@ -993,7 +1029,7 @@ void doNotOptimizeAway(T const& val); #else // These assembly magic is directly from what Google Benchmark is doing. I have previously used what facebook's folly was doing, but -// this seemd to have compilation problems in some cases. Google Benchmark seemed to be the most well tested anyways. +// this seemed to have compilation problems in some cases. Google Benchmark seemed to be the most well tested anyways. // see https://github.com/google/benchmark/blob/master/include/benchmark/benchmark.h#L307 template void doNotOptimizeAway(T const& val) { @@ -1019,7 +1055,11 @@ void doNotOptimizeAway(T& val) { ANKERL_NANOBENCH(IGNORE_EFFCPP_PUSH) class IterationLogic { public: - explicit IterationLogic(Bench const& config) noexcept; + explicit IterationLogic(Bench const& bench); + IterationLogic(IterationLogic&&) = delete; + IterationLogic& operator=(IterationLogic&&) = delete; + IterationLogic(IterationLogic const&) = delete; + IterationLogic& operator=(IterationLogic const&) = delete; ~IterationLogic(); ANKERL_NANOBENCH(NODISCARD) uint64_t numIters() const noexcept; @@ -1036,7 +1076,9 @@ ANKERL_NANOBENCH(IGNORE_PADDED_PUSH) class PerformanceCounters { public: PerformanceCounters(PerformanceCounters const&) = delete; + PerformanceCounters(PerformanceCounters&&) = delete; PerformanceCounters& operator=(PerformanceCounters const&) = delete; + PerformanceCounters& operator=(PerformanceCounters&&) = delete; PerformanceCounters(); ~PerformanceCounters(); @@ -1081,11 +1123,11 @@ class BigO { : BigO(bigOName, mapRangeMeasure(rangeMeasure, rangeToN)) {} template - BigO(std::string const& bigOName, RangeMeasure const& rangeMeasure, Op rangeToN) - : BigO(bigOName, mapRangeMeasure(rangeMeasure, rangeToN)) {} + BigO(std::string bigOName, RangeMeasure const& rangeMeasure, Op rangeToN) + : BigO(std::move(bigOName), mapRangeMeasure(rangeMeasure, rangeToN)) {} BigO(char const* bigOName, RangeMeasure const& scaledRangeMeasure); - BigO(std::string const& bigOName, RangeMeasure const& scaledRangeMeasure); + BigO(std::string bigOName, RangeMeasure const& scaledRangeMeasure); ANKERL_NANOBENCH(NODISCARD) std::string const& name() const noexcept; ANKERL_NANOBENCH(NODISCARD) double constant() const noexcept; ANKERL_NANOBENCH(NODISCARD) double normalizedRootMeanSquare() const noexcept; @@ -1127,7 +1169,7 @@ uint64_t Rng::operator()() noexcept { ANKERL_NANOBENCH_NO_SANITIZE("integer", "undefined") uint32_t Rng::bounded(uint32_t range) noexcept { - uint64_t r32 = static_cast(operator()()); + uint64_t const r32 = static_cast(operator()()); auto multiresult = r32 * range; return static_cast(multiresult >> 32U); } @@ -1136,18 +1178,23 @@ double Rng::uniform01() noexcept { auto i = (UINT64_C(0x3ff) << 52U) | (operator()() >> 12U); // can't use union in c++ here for type puning, it's undefined behavior. // std::memcpy is optimized anyways. - double d; + double d{}; std::memcpy(&d, &i, sizeof(double)); return d - 1.0; } template void Rng::shuffle(Container& container) noexcept { - auto size = static_cast(container.size()); - for (auto i = size; i > 1U; --i) { + auto i = container.size(); + while (i > 1U) { using std::swap; - auto p = bounded(i); // number in [0, i) - swap(container[i - 1], container[p]); + auto n = operator()(); + // using decltype(i) instead of size_t to be compatible to containers with 32bit index (see #80) + auto b1 = static_cast((static_cast(n) * static_cast(i)) >> 32U); + swap(container[--i], container[b1]); + + auto b2 = static_cast(((n >> 32U) * static_cast(i)) >> 32U); + swap(container[--i], container[b2]); } } @@ -1165,11 +1212,11 @@ Bench& Bench::run(Op&& op) { while (auto n = iterationLogic.numIters()) { pc.beginMeasure(); - Clock::time_point before = Clock::now(); + Clock::time_point const before = Clock::now(); while (n-- > 0) { op(); } - Clock::time_point after = Clock::now(); + Clock::time_point const after = Clock::now(); pc.endMeasure(); pc.updateResults(iterationLogic.numIters()); iterationLogic.add(after - before, pc); @@ -1270,7 +1317,6 @@ void doNotOptimizeAway(T const& val) { # include # include # include -# include # endif // declarations /////////////////////////////////////////////////////////////////////////////////// @@ -1436,31 +1482,37 @@ struct Node { template // NOLINTNEXTLINE(hicpp-avoid-c-arrays,modernize-avoid-c-arrays,cppcoreguidelines-avoid-c-arrays) bool operator==(char const (&str)[N]) const noexcept { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) return static_cast(std::distance(begin, end) + 1) == N && 0 == strncmp(str, begin, N - 1); } }; ANKERL_NANOBENCH(IGNORE_PADDED_POP) +// NOLINTNEXTLINE(misc-no-recursion) static std::vector parseMustacheTemplate(char const** tpl) { std::vector nodes; while (true) { - auto begin = std::strstr(*tpl, "{{"); - auto end = begin; + auto const* begin = std::strstr(*tpl, "{{"); + auto const* end = begin; if (begin != nullptr) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) begin += 2; end = std::strstr(begin, "}}"); } if (begin == nullptr || end == nullptr) { // nothing found, finish node + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) nodes.emplace_back(Node{*tpl, *tpl + std::strlen(*tpl), std::vector{}, Node::Type::content}); return nodes; } + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) nodes.emplace_back(Node{*tpl, begin - 2, std::vector{}, Node::Type::content}); // we found a tag + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) *tpl = end + 2; switch (*begin) { case '/': @@ -1468,10 +1520,12 @@ static std::vector parseMustacheTemplate(char const** tpl) { return nodes; case '#': + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) nodes.emplace_back(Node{begin + 1, end, parseMustacheTemplate(tpl), Node::Type::section}); break; case '^': + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) nodes.emplace_back(Node{begin + 1, end, parseMustacheTemplate(tpl), Node::Type::inverted_section}); break; @@ -1484,8 +1538,8 @@ static std::vector parseMustacheTemplate(char const** tpl) { static bool generateFirstLast(Node const& n, size_t idx, size_t size, std::ostream& out) { ANKERL_NANOBENCH_LOG("n.type=" << static_cast(n.type)); - bool matchFirst = n == "-first"; - bool matchLast = n == "-last"; + bool const matchFirst = n == "-first"; + bool const matchLast = n == "-last"; if (!matchFirst && !matchLast) { return false; } @@ -1518,7 +1572,7 @@ static bool matchCmdArgs(std::string const& str, std::vector& match matchResult.emplace_back(str.substr(0, idxOpen)); // split by comma - matchResult.emplace_back(std::string{}); + matchResult.emplace_back(); for (size_t i = idxOpen + 1; i != idxClose; ++i) { if (str[i] == ' ' || str[i] == '\t') { // skip whitespace @@ -1526,7 +1580,7 @@ static bool matchCmdArgs(std::string const& str, std::vector& match } if (str[i] == ',') { // got a comma => new string - matchResult.emplace_back(std::string{}); + matchResult.emplace_back(); continue; } // no whitespace no comma, append @@ -1541,49 +1595,63 @@ static bool generateConfigTag(Node const& n, Config const& config, std::ostream& if (n == "title") { out << config.mBenchmarkTitle; return true; - } else if (n == "name") { + } + if (n == "name") { out << config.mBenchmarkName; return true; - } else if (n == "unit") { + } + if (n == "unit") { out << config.mUnit; return true; - } else if (n == "batch") { + } + if (n == "batch") { out << config.mBatch; return true; - } else if (n == "complexityN") { + } + if (n == "complexityN") { out << config.mComplexityN; return true; - } else if (n == "epochs") { + } + if (n == "epochs") { out << config.mNumEpochs; return true; - } else if (n == "clockResolution") { + } + if (n == "clockResolution") { out << d(detail::clockResolution()); return true; - } else if (n == "clockResolutionMultiple") { + } + if (n == "clockResolutionMultiple") { out << config.mClockResolutionMultiple; return true; - } else if (n == "maxEpochTime") { + } + if (n == "maxEpochTime") { out << d(config.mMaxEpochTime); return true; - } else if (n == "minEpochTime") { + } + if (n == "minEpochTime") { out << d(config.mMinEpochTime); return true; - } else if (n == "minEpochIterations") { + } + if (n == "minEpochIterations") { out << config.mMinEpochIterations; return true; - } else if (n == "epochIterations") { + } + if (n == "epochIterations") { out << config.mEpochIterations; return true; - } else if (n == "warmup") { + } + if (n == "warmup") { out << config.mWarmup; return true; - } else if (n == "relative") { + } + if (n == "relative") { out << config.mIsRelative; return true; } return false; } +// NOLINTNEXTLINE(readability-function-cognitive-complexity) static std::ostream& generateResultTag(Node const& n, Result const& r, std::ostream& out) { if (generateConfigTag(n, r.config(), out)) { return out; @@ -1596,6 +1664,10 @@ static std::ostream& generateResultTag(Node const& n, Result const& r, std::ostr std::vector matchResult; if (matchCmdArgs(std::string(n.begin, n.end), matchResult)) { if (matchResult.size() == 2) { + if (matchResult[0] == "context") { + return out << r.context(matchResult[1]); + } + auto m = Result::fromString(matchResult[1]); if (m == Result::Measure::_size) { return out << 0.0; @@ -1712,7 +1784,7 @@ template T parseFile(std::string const& filename); void gatherStabilityInformation(std::vector& warnings, std::vector& recommendations); -void printStabilityInformationOnce(std::ostream* os); +void printStabilityInformationOnce(std::ostream* outStream); // remembers the last table settings used. When it changes, a new table header is automatically written for the new entry. uint64_t& singletonHeaderHash() noexcept; @@ -1779,13 +1851,13 @@ class Number { }; // helper replacement for std::to_string of signed/unsigned numbers so we are locale independent -std::string to_s(uint64_t s); +std::string to_s(uint64_t n); std::ostream& operator<<(std::ostream& os, Number const& n); class MarkDownColumn { public: - MarkDownColumn(int w, int prec, std::string const& tit, std::string const& suff, double val); + MarkDownColumn(int w, int prec, std::string tit, std::string suff, double val); std::string title() const; std::string separator() const; std::string invalid() const; @@ -1823,8 +1895,9 @@ std::ostream& operator<<(std::ostream& os, MarkDownCode const& mdCode); namespace ankerl { namespace nanobench { +// NOLINTNEXTLINE(readability-function-cognitive-complexity) void render(char const* mustacheTemplate, std::vector const& results, std::ostream& out) { - detail::fmt::StreamStateRestorer restorer(out); + detail::fmt::StreamStateRestorer const restorer(out); out.precision(std::numeric_limits::digits10); auto nodes = templates::parseMustacheTemplate(&mustacheTemplate); @@ -1914,7 +1987,7 @@ void doNotOptimizeAwaySink(void const*) {} template T parseFile(std::string const& filename) { - std::ifstream fin(filename); + std::ifstream fin(filename); // NOLINT(misc-const-correctness) T num{}; fin >> num; return num; @@ -1925,20 +1998,20 @@ char const* getEnv(char const* name) { # pragma warning(push) # pragma warning(disable : 4996) // getenv': This function or variable may be unsafe. # endif - return std::getenv(name); + return std::getenv(name); // NOLINT(concurrency-mt-unsafe) # if defined(_MSC_VER) # pragma warning(pop) # endif } bool isEndlessRunning(std::string const& name) { - auto endless = getEnv("NANOBENCH_ENDLESS"); + auto const* const endless = getEnv("NANOBENCH_ENDLESS"); return nullptr != endless && endless == name; } // True when environment variable NANOBENCH_SUPPRESS_WARNINGS is either not set at all, or set to "0" bool isWarningsEnabled() { - auto suppression = getEnv("NANOBENCH_SUPPRESS_WARNINGS"); + auto const* const suppression = getEnv("NANOBENCH_SUPPRESS_WARNINGS"); return nullptr == suppression || suppression == std::string("0"); } @@ -1946,11 +2019,11 @@ void gatherStabilityInformation(std::vector& warnings, std::vector< warnings.clear(); recommendations.clear(); - bool recommendCheckFlags = false; - # if defined(DEBUG) warnings.emplace_back("DEBUG defined"); - recommendCheckFlags = true; + bool const recommendCheckFlags = true; +# else + bool const recommendCheckFlags = false; # endif bool recommendPyPerf = false; @@ -2000,7 +2073,7 @@ void gatherStabilityInformation(std::vector& warnings, std::vector< void printStabilityInformationOnce(std::ostream* outStream) { static bool shouldPrint = true; - if (shouldPrint && outStream && isWarningsEnabled()) { + if (shouldPrint && (nullptr != outStream) && isWarningsEnabled()) { auto& os = *outStream; shouldPrint = false; std::vector warnings; @@ -2050,7 +2123,7 @@ Clock::duration calcClockResolution(size_t numEvaluations) noexcept { // Calculates clock resolution once, and remembers the result Clock::duration clockResolution() noexcept { - static Clock::duration sResolution = calcClockResolution(20); + static Clock::duration const sResolution = calcClockResolution(20); return sResolution; } @@ -2183,6 +2256,7 @@ struct IterationLogic::Impl { << ", mState=" << static_cast(mState)); } + // NOLINTNEXTLINE(readability-function-cognitive-complexity) void showResult(std::string const& errorMessage) const { ANKERL_NANOBENCH_LOG(errorMessage); @@ -2208,7 +2282,7 @@ struct IterationLogic::Impl { rMedian / (mBench.timeUnit().count() * mBench.batch())); columns.emplace_back(22, 2, mBench.unit() + "/s", "", rMedian <= 0.0 ? 0.0 : mBench.batch() / rMedian); - double rErrorMedian = mResult.medianAbsolutePercentError(Result::Measure::elapsed); + double const rErrorMedian = mResult.medianAbsolutePercentError(Result::Measure::elapsed); columns.emplace_back(10, 1, "err%", "%", rErrorMedian * 100.0); double rInsMedian = -1.0; @@ -2226,7 +2300,7 @@ struct IterationLogic::Impl { columns.emplace_back(9, 3, "IPC", "", rCycMedian <= 0.0 ? 0.0 : rInsMedian / rCycMedian); } if (mBench.performanceCounters() && mResult.has(Result::Measure::branchinstructions)) { - double rBraMedian = mResult.median(Result::Measure::branchinstructions); + double const rBraMedian = mResult.median(Result::Measure::branchinstructions); columns.emplace_back(17, 2, "bra/" + mBench.unit(), "", rBraMedian / mBench.batch()); if (mResult.has(Result::Measure::branchmisses)) { double p = 0.0; @@ -2299,25 +2373,22 @@ struct IterationLogic::Impl { return elapsed * 3 >= mTargetRuntimePerEpoch * 2; } - uint64_t mNumIters = 1; - Bench const& mBench; - std::chrono::nanoseconds mTargetRuntimePerEpoch{}; - Result mResult; - Rng mRng{123}; - std::chrono::nanoseconds mTotalElapsed{}; - uint64_t mTotalNumIters = 0; - - State mState = State::upscaling_runtime; + uint64_t mNumIters = 1; // NOLINT(misc-non-private-member-variables-in-classes) + Bench const& mBench; // NOLINT(misc-non-private-member-variables-in-classes) + std::chrono::nanoseconds mTargetRuntimePerEpoch{}; // NOLINT(misc-non-private-member-variables-in-classes) + Result mResult; // NOLINT(misc-non-private-member-variables-in-classes) + Rng mRng{123}; // NOLINT(misc-non-private-member-variables-in-classes) + std::chrono::nanoseconds mTotalElapsed{}; // NOLINT(misc-non-private-member-variables-in-classes) + uint64_t mTotalNumIters = 0; // NOLINT(misc-non-private-member-variables-in-classes) + State mState = State::upscaling_runtime; // NOLINT(misc-non-private-member-variables-in-classes) }; ANKERL_NANOBENCH(IGNORE_PADDED_POP) -IterationLogic::IterationLogic(Bench const& bench) noexcept +IterationLogic::IterationLogic(Bench const& bench) : mPimpl(new Impl(bench)) {} IterationLogic::~IterationLogic() { - if (mPimpl) { - delete mPimpl; - } + delete mPimpl; } uint64_t IterationLogic::numIters() const noexcept { @@ -2344,11 +2415,16 @@ class LinuxPerformanceCounters { , correctMeasuringOverhead(correctMeasuringOverhead_) , correctLoopOverhead(correctLoopOverhead_) {} - uint64_t* targetValue{}; - bool correctMeasuringOverhead{}; - bool correctLoopOverhead{}; + uint64_t* targetValue{}; // NOLINT(misc-non-private-member-variables-in-classes) + bool correctMeasuringOverhead{}; // NOLINT(misc-non-private-member-variables-in-classes) + bool correctLoopOverhead{}; // NOLINT(misc-non-private-member-variables-in-classes) }; + LinuxPerformanceCounters() = default; + LinuxPerformanceCounters(LinuxPerformanceCounters const&) = delete; + LinuxPerformanceCounters(LinuxPerformanceCounters&&) = delete; + LinuxPerformanceCounters& operator=(LinuxPerformanceCounters const&) = delete; + LinuxPerformanceCounters& operator=(LinuxPerformanceCounters&&) = delete; ~LinuxPerformanceCounters(); // quick operation @@ -2370,13 +2446,13 @@ class LinuxPerformanceCounters { return; } - // NOLINTNEXTLINE(hicpp-signed-bitwise) + // NOLINTNEXTLINE(hicpp-signed-bitwise,cppcoreguidelines-pro-type-vararg) mHasError = -1 == ioctl(mFd, PERF_EVENT_IOC_RESET, PERF_IOC_FLAG_GROUP); if (mHasError) { return; } - // NOLINTNEXTLINE(hicpp-signed-bitwise) + // NOLINTNEXTLINE(hicpp-signed-bitwise,cppcoreguidelines-pro-type-vararg) mHasError = -1 == ioctl(mFd, PERF_EVENT_IOC_ENABLE, PERF_IOC_FLAG_GROUP); } @@ -2385,7 +2461,7 @@ class LinuxPerformanceCounters { return; } - // NOLINTNEXTLINE(hicpp-signed-bitwise) + // NOLINTNEXTLINE(hicpp-signed-bitwise,cppcoreguidelines-pro-type-vararg) mHasError = (-1 == ioctl(mFd, PERF_EVENT_IOC_DISABLE, PERF_IOC_FLAG_GROUP)); if (mHasError) { return; @@ -2406,9 +2482,9 @@ class LinuxPerformanceCounters { ANKERL_NANOBENCH_NO_SANITIZE("integer", "undefined") static inline uint32_t mix(uint32_t x) noexcept { - x ^= x << 13; - x ^= x >> 17; - x ^= x << 5; + x ^= x << 13U; + x ^= x >> 17U; + x ^= x << 5U; return x; } @@ -2448,7 +2524,7 @@ class LinuxPerformanceCounters { // marsaglia's xorshift: mov, sal/shr, xor. Times 3. // This has the nice property that the compiler doesn't seem to be able to optimize multiple calls any further. // see https://godbolt.org/z/49RVQ5 - uint64_t const numIters = 100000U + (std::random_device{}() & 3); + uint64_t const numIters = 100000U + (std::random_device{}() & 3U); uint64_t n = numIters; uint32_t x = 1234567; @@ -2582,6 +2658,7 @@ bool LinuxPerformanceCounters::monitor(uint32_t type, uint64_t eventid, Target t const unsigned long flags = 0; # endif + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) auto fd = static_cast(syscall(__NR_perf_event_open, &pea, pid, cpu, mFd, flags)); if (-1 == fd) { return false; @@ -2591,7 +2668,7 @@ bool LinuxPerformanceCounters::monitor(uint32_t type, uint64_t eventid, Target t mFd = fd; } uint64_t id = 0; - // NOLINTNEXTLINE(hicpp-signed-bitwise) + // NOLINTNEXTLINE(hicpp-signed-bitwise,cppcoreguidelines-pro-type-vararg) if (-1 == ioctl(fd, PERF_EVENT_IOC_ID, &id)) { // couldn't get id return false; @@ -2639,9 +2716,8 @@ PerformanceCounters::PerformanceCounters() } PerformanceCounters::~PerformanceCounters() { - if (nullptr != mPc) { - delete mPc; - } + // no need to check for nullptr, delete nullptr has no effect + delete mPc; } void PerformanceCounters::beginMeasure() { @@ -2721,7 +2797,7 @@ Number::Number(int width, int precision, double value) , mValue(value) {} std::ostream& Number::write(std::ostream& os) const { - StreamStateRestorer restorer(os); + StreamStateRestorer const restorer(os); os.imbue(std::locale(os.getloc(), new NumSep(','))); os << std::setw(mWidth) << std::setprecision(mPrecision) << std::fixed << mValue; return os; @@ -2747,11 +2823,11 @@ std::ostream& operator<<(std::ostream& os, Number const& n) { return n.write(os); } -MarkDownColumn::MarkDownColumn(int w, int prec, std::string const& tit, std::string const& suff, double val) +MarkDownColumn::MarkDownColumn(int w, int prec, std::string tit, std::string suff, double val) : mWidth(w) , mPrecision(prec) - , mTitle(tit) - , mSuffix(suff) + , mTitle(std::move(tit)) + , mSuffix(std::move(suff)) , mValue(val) {} std::string MarkDownColumn::title() const { @@ -2785,7 +2861,7 @@ std::string MarkDownColumn::value() const { MarkDownCode::MarkDownCode(std::string const& what) { mWhat.reserve(what.size() + 2); mWhat.push_back('`'); - for (char c : what) { + for (char const c : what) { mWhat.push_back(c); if ('`' == c) { mWhat.push_back('`'); @@ -2808,14 +2884,14 @@ std::ostream& operator<<(std::ostream& os, MarkDownCode const& mdCode) { Config::Config() = default; Config::~Config() = default; Config& Config::operator=(Config const&) = default; -Config& Config::operator=(Config&&) = default; +Config& Config::operator=(Config&&) noexcept = default; Config::Config(Config const&) = default; Config::Config(Config&&) noexcept = default; // provide implementation here so it's only generated once Result::~Result() = default; Result& Result::operator=(Result const&) = default; -Result& Result::operator=(Result&&) = default; +Result& Result::operator=(Result&&) noexcept = default; Result::Result(Result const&) = default; Result::Result(Result&&) noexcept = default; @@ -2827,15 +2903,15 @@ inline constexpr typename std::underlying_type::type u(T val) noexcept { } // namespace detail // Result returned after a benchmark has finished. Can be used as a baseline for relative(). -Result::Result(Config const& benchmarkConfig) - : mConfig(benchmarkConfig) +Result::Result(Config benchmarkConfig) + : mConfig(std::move(benchmarkConfig)) , mNameToMeasurements{detail::u(Result::Measure::_size)} {} void Result::add(Clock::duration totalElapsed, uint64_t iters, detail::PerformanceCounters const& pc) { using detail::d; using detail::u; - double dIters = d(iters); + double const dIters = d(iters); mNameToMeasurements[u(Result::Measure::iterations)].push_back(dIters); mNameToMeasurements[u(Result::Measure::elapsed)].push_back(d(totalElapsed) / dIters); @@ -2987,27 +3063,41 @@ double Result::maximum(Measure m) const noexcept { return *std::max_element(data.begin(), data.end()); } +std::string const& Result::context(char const* variableName) const { + return mConfig.mContext.at(variableName); +} + +std::string const& Result::context(std::string const& variableName) const { + return mConfig.mContext.at(variableName); +} + Result::Measure Result::fromString(std::string const& str) { if (str == "elapsed") { return Measure::elapsed; - } else if (str == "iterations") { + } + if (str == "iterations") { return Measure::iterations; - } else if (str == "pagefaults") { + } + if (str == "pagefaults") { return Measure::pagefaults; - } else if (str == "cpucycles") { + } + if (str == "cpucycles") { return Measure::cpucycles; - } else if (str == "contextswitches") { + } + if (str == "contextswitches") { return Measure::contextswitches; - } else if (str == "instructions") { + } + if (str == "instructions") { return Measure::instructions; - } else if (str == "branchinstructions") { + } + if (str == "branchinstructions") { return Measure::branchinstructions; - } else if (str == "branchmisses") { + } + if (str == "branchmisses") { return Measure::branchmisses; - } else { - // not found, return _size - return Measure::_size; } + // not found, return _size + return Measure::_size; } // Configuration of a microbenchmark. @@ -3015,8 +3105,8 @@ Bench::Bench() { mConfig.mOut = &std::cout; } -Bench::Bench(Bench&&) = default; -Bench& Bench::operator=(Bench&&) = default; +Bench::Bench(Bench&&) noexcept = default; +Bench& Bench::operator=(Bench&&) noexcept = default; Bench::Bench(Bench const&) = default; Bench& Bench::operator=(Bench const&) = default; Bench::~Bench() noexcept = default; @@ -3114,6 +3204,21 @@ std::string const& Bench::name() const noexcept { return mConfig.mBenchmarkName; } +Bench& Bench::context(char const* variableName, char const* variableValue) { + mConfig.mContext[variableName] = variableValue; + return *this; +} + +Bench& Bench::context(std::string const& variableName, std::string const& variableValue) { + mConfig.mContext[variableName] = variableValue; + return *this; +} + +Bench& Bench::clearContext() { + mConfig.mContext.clear(); + return *this; +} + // Number of epochs to evaluate. The reported result will be the median of evaluation of each epoch. Bench& Bench::epochs(size_t numEpochs) noexcept { mConfig.mNumEpochs = numEpochs; @@ -3295,27 +3400,27 @@ BigO::RangeMeasure BigO::collectRangeMeasure(std::vector const& results) return rangeMeasure; } -BigO::BigO(std::string const& bigOName, RangeMeasure const& rangeMeasure) - : mName(bigOName) { +BigO::BigO(std::string bigOName, RangeMeasure const& rangeMeasure) + : mName(std::move(bigOName)) { // estimate the constant factor double sumRangeMeasure = 0.0; double sumRangeRange = 0.0; - for (size_t i = 0; i < rangeMeasure.size(); ++i) { - sumRangeMeasure += rangeMeasure[i].first * rangeMeasure[i].second; - sumRangeRange += rangeMeasure[i].first * rangeMeasure[i].first; + for (const auto& rm : rangeMeasure) { + sumRangeMeasure += rm.first * rm.second; + sumRangeRange += rm.first * rm.first; } mConstant = sumRangeMeasure / sumRangeRange; // calculate root mean square double err = 0.0; double sumMeasure = 0.0; - for (size_t i = 0; i < rangeMeasure.size(); ++i) { - auto diff = mConstant * rangeMeasure[i].first - rangeMeasure[i].second; + for (const auto& rm : rangeMeasure) { + auto diff = mConstant * rm.first - rm.second; err += diff * diff; - sumMeasure += rangeMeasure[i].second; + sumMeasure += rm.second; } auto n = static_cast(rangeMeasure.size()); @@ -3347,7 +3452,7 @@ std::ostream& operator<<(std::ostream& os, BigO const& bigO) { } std::ostream& operator<<(std::ostream& os, std::vector const& bigOs) { - detail::fmt::StreamStateRestorer restorer(os); + detail::fmt::StreamStateRestorer const restorer(os); os << std::endl << "| coefficient | err% | complexity" << std::endl << "|--------------:|-------:|------------" << std::endl; for (auto const& bigO : bigOs) { os << "|" << std::setw(14) << std::setprecision(7) << std::scientific << bigO.constant() << " "; From 97858384ec8451f8d11b9ac5e1a6a61c03505f1e Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 7 Feb 2023 11:53:41 +0000 Subject: [PATCH 16/21] Merge bitcoin/bitcoin#21995: build: Make dependency package archive timestamps deterministic 6ebe57622cb70df529879b15f291166177f2827c build: Make dependency package archive timestamps deterministic (Hennadii Stepanov) Pull request description: This PR makes testing changes like bitcoin/bitcoin#20641, bitcoin/bitcoin#21593, bitcoin/bitcoin#22142, bitcoin/bitcoin#24279, bitcoin/bitcoin#24285 as easy as comparing hashes. With this PR: ``` $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-ca950bd6d13.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-b537c466dcb.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-e13b2bdd8b8.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-82255b84667.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-c43fc4cf2f6.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-663de0dc628.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-986926343e2.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bec6a4d3299.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-8ae81bab6f4.tar.gz ``` As an example, here is an evidence that bitcoin/bitcoin#24279 is a strict refactoring change: ``` $ git fetch origin pull/24279/head $ git cherry-pick 706026838d917a3d853e03e83db040f1fd4aeb74 $ git cherry-pick 3f90ddea8a6a2061cfb347a1d77df2c0a6fa238c $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-c7faf31d5ca.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-1af3dd1d99e.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-6228a9f8534.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-41aa6194ecc.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-6a93027769c.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-d40cb2d45c9.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-120c3cb745d.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bbd4d813c69.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-df0858a19d2.tar.gz ``` ACKs for top commit: TheCharlatan: Code review ACK 6ebe57622cb70df529879b15f291166177f2827c Tree-SHA512: 20e0222781f5dcb50126c11677d0671bcdd7be144b2e528c75a02983acc494206552fb35039697ccd094de27a21b3fb439e9965c34feb8a6d74627fa20a9a5e7 --- depends/funcs.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/depends/funcs.mk b/depends/funcs.mk index 6e12aae2ac203..76b205ae6d3a6 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -241,7 +241,9 @@ $($(1)_postprocessed): | $($(1)_staged) touch $$@ $($(1)_cached): | $($(1)_dependencies) $($(1)_postprocessed) echo Caching $(1)... - cd $$($(1)_staging_dir)/$(host_prefix); find . | sort | $(build_TAR) --no-recursion -czf $$($(1)_staging_dir)/$$(@F) -T - + cd $$($(1)_staging_dir)/$(host_prefix); \ + find . ! -name '.stamp_postprocessed' -print0 | TZ=UTC xargs -0r touch -h -m -t 200001011200; \ + find . ! -name '.stamp_postprocessed' | LC_ALL=C sort | $(build_TAR) --numeric-owner --no-recursion -czf $$($(1)_staging_dir)/$$(@F) -T - mkdir -p $$(@D) rm -rf $$(@D) && mkdir -p $$(@D) mv $$($(1)_staging_dir)/$$(@F) $$(@) From 6889a8db29a787266cab23d39d93df3f83c2bb81 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 8 Feb 2023 12:49:36 +0100 Subject: [PATCH 17/21] Merge bitcoin/bitcoin#27056: doc: use arch agnostic clang path in fuzzing doc (macOS) b49e19ccd9a50053c69cd42bae1b44df07890cfd doc: use arch agnostic clang path in fuzzing doc (macOS) (fanquake) Pull request description: The current path will only work for clang installed via brew on x86_64 macOS. ACKs for top commit: hebasto: ACK b49e19ccd9a50053c69cd42bae1b44df07890cfd, similar to 702836530ffa351e863b1b1300fd2e559a14ef23. Tree-SHA512: 8ae4845e1953d5a7178f2b422e2241af1057d8cce1ab79da65df0cd068456dbf85da3489355f81fc4ee09ba602a4b53e989e2dc02476b4abf6c5b3bc3e96473b --- doc/fuzzing.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/fuzzing.md b/doc/fuzzing.md index 614f475d41abd..483d1e52105a9 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -129,10 +129,10 @@ You may also need to take care of giving the correct path for `clang` and `clang++`, like `CC=/path/to/clang CXX=/path/to/clang++` if the non-systems `clang` does not come first in your path. -Full configure that was tested on macOS Catalina with `brew` installed `llvm`: +Full configure that was tested on macOS with `brew` installed `llvm`: ```sh -./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm +./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined --disable-asm CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++ ``` Read the [libFuzzer documentation](https://llvm.org/docs/LibFuzzer.html) for more information. This [libFuzzer tutorial](https://github.com/google/fuzzing/blob/master/tutorial/libFuzzerTutorial.md) might also be of interest. From f115d9c27f6b18f5fe1ad3d70c356f4831b31697 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 8 Feb 2023 14:58:16 +0000 Subject: [PATCH 18/21] Merge bitcoin/bitcoin#27061: doc: Document affected gcc versions for -fstack-reuse=none workaround fa83005a2618abc18f9cd6af96f19fc28aba68d8 doc: Document affected gcc versions for -fstack-reuse=none workaround (MarcoFalke) Pull request description: gcc version(s) 11 and prior won't be fixed, looking at the activity in the bug report. So it seems best to just document gcc 12.1+ as fixed, so that in the future the workaround can be removed once the minimum compiler is gcc12.1. ACKs for top commit: fanquake: ACK fa83005a2618abc18f9cd6af96f19fc28aba68d8 hebasto: re-ACK fa83005a2618abc18f9cd6af96f19fc28aba68d8 Tree-SHA512: a19723457eb1925196828a5fafd4e7f75a04f86ffae63cb86679d732c662fd1a22e17fe3c69195a97438ff189ba3ff681be3650cf99aa195d7a3e89cd8ee137c --- configure.ac | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 50441b50fb199..e24f809ffb2a6 100644 --- a/configure.ac +++ b/configure.ac @@ -957,7 +957,9 @@ if test x$TARGET_OS != xwindows; then AX_CHECK_COMPILE_FLAG([-fPIC],[PIC_FLAGS="-fPIC"]) fi -dnl All versions of gcc that we commonly use for building are subject to bug +dnl Versions of gcc prior to 12.1 (commit +dnl https://github.com/gcc-mirror/gcc/commit/551aa75778a4c5165d9533cd447c8fc822f583e1) +dnl are subject to a bug, see the gccbug_90348 test case and dnl https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348. To work around that, set dnl -fstack-reuse=none for all gcc builds. (Only gcc understands this flag) AX_CHECK_COMPILE_FLAG([-fstack-reuse=none],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-reuse=none"]) From a620cccd8179d9c9a43941f1e64852576041e93a Mon Sep 17 00:00:00 2001 From: merge-script <90386131+bitcoin-core-merge-script@users.noreply.github.com> Date: Mon, 13 Feb 2023 11:51:03 +0100 Subject: [PATCH 19/21] Merge bitcoin/bitcoin#26970: test: fix immediate tx relay in wallet_groups.py ab4efad51b9ba276ffeb6871931e13772493f7cc test: fix immediate tx relay in wallet_groups.py (Sebastian Falbesoner) Pull request description: In the functional test wallet_groups.py we whitelist peers on all nodes (`-whitelist=noban@127.0.0.1`) to enable immediate tx relay for fast mempool synchronization. However, considering that this setting only applies to inbound peers and the default test topology looks like this: ``` node0 <--- node1 <---- node2 <--- ... <-- nodeN ``` txs propagate fast only from lower- to higher-numbered nodes (i.e. "left to right" in the above diagram) and take long from higher- to lower-numbered nodes ("right to left") since in the latter direction we only have outbound peers, where the trickle relay is still active. As a consequence, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long. This PR fixes this by simply adding another connection from node0 to the last node, leading to a ~2-3x speedup (5 runs measured via `time ./test/functional/wallet_groups.py` are shown): ``` master: 0m53.31s real 0m08.22s user 0m05.60s system 0m32.85s real 0m07.44s user 0m04.08s system 0m46.40s real 0m09.18s user 0m04.23s system 0m46.96s real 0m11.10s user 0m05.74s system 0m57.23s real 0m10.53s user 0m05.59s system PR: 0m19.64s real 0m09.58s user 0m05.50s system 0m18.05s real 0m07.77s user 0m04.03s system 0m18.99s real 0m07.90s user 0m04.25s system 0m17.49s real 0m07.56s user 0m03.92s system 0m18.11s real 0m07.74s user 0m03.88s system ``` Note that in most tests this is not a problem since txs very often originate from node0. ACKs for top commit: brunoerg: utACK ab4efad51b9ba276ffeb6871931e13772493f7cc Tree-SHA512: 12675357e6eb5a18383f2bfe719a184c0790863b37a98749d8e757dd5dc3a36212e16a81f0a192340c11b793eda00db359c7011f46f7c27e3a093af4f5b62147 --- test/functional/wallet_groups.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index b2b85a566b68e..3eb6a8fa3c761 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -33,6 +33,11 @@ def skip_test_if_missing_module(self): def run_test(self): self.log.info("Setting up") + # To take full use of immediate tx relay, all nodes need to be reachable + # via inbound peers, i.e. connect first to last to close the circle + # (the default test network topology looks like this: + # node0 <-- node1 <-- node2 <-- node3 <-- node4 <-- node5) + self.connect_nodes(0, self.num_nodes - 1) # Mine some coins self.generate(self.nodes[0], COINBASE_MATURITY + 1) From 417c86b949de65c9255a9de40c09a740a264d35d Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 12 Sep 2023 15:00:18 +0100 Subject: [PATCH 20/21] Merge bitcoin/bitcoin#28105: doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCC fabb419a3caafc00fbbc533fee14fab7a5d2a2c6 doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCC (MarcoFalke) Pull request description: This is a follow-up to commit 7b850bc2a1cd8547a2dbb5a18173f53439601220. While the test case no longer reproduces, the general class of `-fstack-reuse` bugs still exists in all versions of GCC. The workaround can never be removed, unless the whole class of bugs is fixed. ACKs for top commit: fanquake: ACK fabb419a3caafc00fbbc533fee14fab7a5d2a2c6 Tree-SHA512: 566e14fe82d13dda4f7b8cca90c6de75006d14828906b936780716d5b5b31de9b36a904aa7cfc9820ccdfb4d3224a8437f502f25f7230da5abe87c927123f0c8 --- configure.ac | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index e24f809ffb2a6..1b1dd756137e6 100644 --- a/configure.ac +++ b/configure.ac @@ -957,9 +957,8 @@ if test x$TARGET_OS != xwindows; then AX_CHECK_COMPILE_FLAG([-fPIC],[PIC_FLAGS="-fPIC"]) fi -dnl Versions of gcc prior to 12.1 (commit -dnl https://github.com/gcc-mirror/gcc/commit/551aa75778a4c5165d9533cd447c8fc822f583e1) -dnl are subject to a bug, see the gccbug_90348 test case and +dnl Currently all versions of gcc are subject to a class of bugs, see the +dnl gccbug_90348 test case (only reproduces on GCC 11 and earlier) and the related bugs of dnl https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348. To work around that, set dnl -fstack-reuse=none for all gcc builds. (Only gcc understands this flag) AX_CHECK_COMPILE_FLAG([-fstack-reuse=none],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-reuse=none"]) From 168e5e4a50560218187ba5d210c21c68456ca57c Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 16 Nov 2023 09:45:25 +0000 Subject: [PATCH 21/21] Merge bitcoin/bitcoin#28877: bench: Update nanobench to 4.3.11 fe434a469534766f18d7560d968deed37193835f bench: Update nanobench to 4.3.11 (TheCharlatan) Pull request description: The newest version fixes the false positive `* Turbo is enabled, CPU frequency will fluctuate` warning on AMD CPUs. The file was directly taken from the release page: https://github.com/martinus/nanobench/releases/tag/v4.3.11. Other changes from the release notes: * Check for failures in parseFile(), perf events tweaks by tommi-cujo in https://github.com/martinus/nanobench/pull/84 * Workaround missing noexcept for std::string move assignment by tommi-cujo in https://github.com/martinus/nanobench/pull/87 * removed the link by martinus in https://github.com/martinus/nanobench/pull/89 * Lots of minor cleanups by martinus in https://github.com/martinus/nanobench/pull/85 * Add linter for version & clang-format. Updated version by martinus in https://github.com/martinus/nanobench/pull/90 ACKs for top commit: fanquake: ACK fe434a469534766f18d7560d968deed37193835f - have not tested. Tree-SHA512: a8f15e1db1d993673e4b295a3bab22e67ee3c9f3c0bcbef28974fe9ff37dbb741967a526088d5b148c8d25c9d57cd3b844238100c17b23038638787461805678 --- src/bench/nanobench.h | 98 ++++++++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 42 deletions(-) diff --git a/src/bench/nanobench.h b/src/bench/nanobench.h index c518b91b6a1b0..4808b866fef4e 100644 --- a/src/bench/nanobench.h +++ b/src/bench/nanobench.h @@ -33,7 +33,7 @@ // see https://semver.org/ #define ANKERL_NANOBENCH_VERSION_MAJOR 4 // incompatible API changes #define ANKERL_NANOBENCH_VERSION_MINOR 3 // backwards-compatible changes -#define ANKERL_NANOBENCH_VERSION_PATCH 10 // backwards-compatible bug fixes +#define ANKERL_NANOBENCH_VERSION_PATCH 11 // backwards-compatible bug fixes /////////////////////////////////////////////////////////////////////////////////////////////////// // public facing api - as minimal as possible @@ -120,6 +120,10 @@ # define ANKERL_NANOBENCH_IS_TRIVIALLY_COPYABLE(...) std::is_trivially_copyable<__VA_ARGS__>::value #endif +// noexcept may be missing for std::string. +// See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58265 +#define ANKERL_NANOBENCH_PRIVATE_NOEXCEPT_STRING_MOVE() std::is_nothrow_move_assignable::value + // declarations /////////////////////////////////////////////////////////////////////////////////// namespace ankerl { @@ -404,7 +408,7 @@ struct Config { Config(); ~Config(); Config& operator=(Config const& other); - Config& operator=(Config&& other) noexcept; + Config& operator=(Config&& other) noexcept(ANKERL_NANOBENCH(NOEXCEPT_STRING_MOVE)); Config(Config const& other); Config(Config&& other) noexcept; }; @@ -430,7 +434,7 @@ class Result { ~Result(); Result& operator=(Result const& other); - Result& operator=(Result&& other) noexcept; + Result& operator=(Result&& other) noexcept(ANKERL_NANOBENCH(NOEXCEPT_STRING_MOVE)); Result(Result const& other); Result(Result&& other) noexcept; @@ -596,7 +600,7 @@ class Rng final { * * @return Vector containing the full state: */ - std::vector state() const; + ANKERL_NANOBENCH(NODISCARD) std::vector state() const; private: static constexpr uint64_t rotl(uint64_t x, unsigned k) noexcept; @@ -628,7 +632,7 @@ class Bench { Bench(); Bench(Bench&& other) noexcept; - Bench& operator=(Bench&& other) noexcept; + Bench& operator=(Bench&& other) noexcept(ANKERL_NANOBENCH(NOEXCEPT_STRING_MOVE)); Bench(Bench const& other); Bench& operator=(Bench const& other); ~Bench() noexcept; @@ -818,7 +822,7 @@ class Bench { * Default is zero, so we are fully relying on clockResolutionMultiple(). In most cases this is exactly what you want. If you see * that the evaluation is unreliable with a high `err%`, you can increase either minEpochTime() or minEpochIterations(). * - * @see maxEpochTim), minEpochIterations + * @see maxEpochTime, minEpochIterations * * @param t Minimum time each epoch should take. */ @@ -1030,7 +1034,7 @@ void doNotOptimizeAway(T const& val); // These assembly magic is directly from what Google Benchmark is doing. I have previously used what facebook's folly was doing, but // this seemed to have compilation problems in some cases. Google Benchmark seemed to be the most well tested anyways. -// see https://github.com/google/benchmark/blob/master/include/benchmark/benchmark.h#L307 +// see https://github.com/google/benchmark/blob/v1.7.1/include/benchmark/benchmark.h#L443-L446 template void doNotOptimizeAway(T const& val) { // NOLINTNEXTLINE(hicpp-no-assembler) @@ -1781,7 +1785,7 @@ bool isEndlessRunning(std::string const& name); bool isWarningsEnabled(); template -T parseFile(std::string const& filename); +T parseFile(std::string const& filename, bool* fail); void gatherStabilityInformation(std::vector& warnings, std::vector& recommendations); void printStabilityInformationOnce(std::ostream* outStream); @@ -1839,7 +1843,7 @@ class Number { public: Number(int width, int precision, double value); Number(int width, int precision, int64_t value); - std::string to_s() const; + ANKERL_NANOBENCH(NODISCARD) std::string to_s() const; private: friend std::ostream& operator<<(std::ostream& os, Number const& n); @@ -1857,11 +1861,11 @@ std::ostream& operator<<(std::ostream& os, Number const& n); class MarkDownColumn { public: - MarkDownColumn(int w, int prec, std::string tit, std::string suff, double val); - std::string title() const; - std::string separator() const; - std::string invalid() const; - std::string value() const; + MarkDownColumn(int w, int prec, std::string tit, std::string suff, double val) noexcept; + ANKERL_NANOBENCH(NODISCARD) std::string title() const; + ANKERL_NANOBENCH(NODISCARD) std::string separator() const; + ANKERL_NANOBENCH(NODISCARD) std::string invalid() const; + ANKERL_NANOBENCH(NODISCARD) std::string value() const; private: int mWidth; @@ -1976,9 +1980,9 @@ PerformanceCounters& performanceCounters() { } // Windows version of doNotOptimizeAway -// see https://github.com/google/benchmark/blob/master/include/benchmark/benchmark.h#L307 -// see https://github.com/facebook/folly/blob/master/folly/Benchmark.h#L280 -// see https://docs.microsoft.com/en-us/cpp/preprocessor/optimize +// see https://github.com/google/benchmark/blob/v1.7.1/include/benchmark/benchmark.h#L514 +// see https://github.com/facebook/folly/blob/v2023.01.30.00/folly/lang/Hint-inl.h#L54-L58 +// see https://learn.microsoft.com/en-us/cpp/preprocessor/optimize # if defined(_MSC_VER) # pragma optimize("", off) void doNotOptimizeAwaySink(void const*) {} @@ -1986,10 +1990,13 @@ void doNotOptimizeAwaySink(void const*) {} # endif template -T parseFile(std::string const& filename) { +T parseFile(std::string const& filename, bool* fail) { std::ifstream fin(filename); // NOLINT(misc-const-correctness) T num{}; fin >> num; + if (fail != nullptr) { + *fail = fin.fail(); + } return num; } @@ -2032,16 +2039,15 @@ void gatherStabilityInformation(std::vector& warnings, std::vector< if (nprocs <= 0) { warnings.emplace_back("couldn't figure out number of processors - no governor, turbo check possible"); } else { - // check frequency scaling for (long id = 0; id < nprocs; ++id) { auto idStr = detail::fmt::to_s(static_cast(id)); auto sysCpu = "/sys/devices/system/cpu/cpu" + idStr; - auto minFreq = parseFile(sysCpu + "/cpufreq/scaling_min_freq"); - auto maxFreq = parseFile(sysCpu + "/cpufreq/scaling_max_freq"); + auto minFreq = parseFile(sysCpu + "/cpufreq/scaling_min_freq", nullptr); + auto maxFreq = parseFile(sysCpu + "/cpufreq/scaling_max_freq", nullptr); if (minFreq != maxFreq) { - auto minMHz = static_cast(minFreq) / 1000.0; - auto maxMHz = static_cast(maxFreq) / 1000.0; + auto minMHz = d(minFreq) / 1000.0; + auto maxMHz = d(maxFreq) / 1000.0; warnings.emplace_back("CPU frequency scaling enabled: CPU " + idStr + " between " + detail::fmt::Number(1, 1, minMHz).to_s() + " and " + detail::fmt::Number(1, 1, maxMHz).to_s() + " MHz"); @@ -2050,13 +2056,15 @@ void gatherStabilityInformation(std::vector& warnings, std::vector< } } - auto currentGovernor = parseFile("/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor"); - if ("performance" != currentGovernor) { + auto fail = false; + auto currentGovernor = parseFile("/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor", &fail); + if (!fail && "performance" != currentGovernor) { warnings.emplace_back("CPU governor is '" + currentGovernor + "' but should be 'performance'"); recommendPyPerf = true; } - if (0 == parseFile("/sys/devices/system/cpu/intel_pstate/no_turbo")) { + auto noTurbo = parseFile("/sys/devices/system/cpu/intel_pstate/no_turbo", &fail); + if (!fail && noTurbo == 0) { warnings.emplace_back("Turbo is enabled, CPU frequency will fluctuate"); recommendPyPerf = true; } @@ -2250,10 +2258,9 @@ struct IterationLogic::Impl { mNumIters = 0; } - ANKERL_NANOBENCH_LOG(mBench.name() << ": " << detail::fmt::Number(20, 3, static_cast(elapsed.count())) << " elapsed, " - << detail::fmt::Number(20, 3, static_cast(mTargetRuntimePerEpoch.count())) - << " target. oldIters=" << oldIters << ", mNumIters=" << mNumIters - << ", mState=" << static_cast(mState)); + ANKERL_NANOBENCH_LOG(mBench.name() << ": " << detail::fmt::Number(20, 3, d(elapsed.count())) << " elapsed, " + << detail::fmt::Number(20, 3, d(mTargetRuntimePerEpoch.count())) << " target. oldIters=" + << oldIters << ", mNumIters=" << mNumIters << ", mState=" << static_cast(mState)); } // NOLINTNEXTLINE(readability-function-cognitive-complexity) @@ -2357,7 +2364,7 @@ struct IterationLogic::Impl { } os << fmt::MarkDownCode(mBench.name()); if (showUnstable) { - auto avgIters = static_cast(mTotalNumIters) / static_cast(mBench.epochs()); + auto avgIters = d(mTotalNumIters) / d(mBench.epochs()); // NOLINTNEXTLINE(bugprone-incorrect-roundings) auto suggestedIters = static_cast(avgIters * 10 + 0.5); @@ -2435,7 +2442,7 @@ class LinuxPerformanceCounters { bool monitor(perf_sw_ids swId, Target target); bool monitor(perf_hw_id hwId, Target target); - bool hasError() const noexcept { + ANKERL_NANOBENCH(NODISCARD) bool hasError() const noexcept { return mHasError; } @@ -2691,16 +2698,23 @@ PerformanceCounters::PerformanceCounters() , mVal() , mHas() { - mHas.pageFaults = mPc->monitor(PERF_COUNT_SW_PAGE_FAULTS, LinuxPerformanceCounters::Target(&mVal.pageFaults, true, false)); + // HW events mHas.cpuCycles = mPc->monitor(PERF_COUNT_HW_REF_CPU_CYCLES, LinuxPerformanceCounters::Target(&mVal.cpuCycles, true, false)); - mHas.contextSwitches = - mPc->monitor(PERF_COUNT_SW_CONTEXT_SWITCHES, LinuxPerformanceCounters::Target(&mVal.contextSwitches, true, false)); + if (!mHas.cpuCycles) { + // Fallback to cycles counter, reference cycles not available in many systems. + mHas.cpuCycles = mPc->monitor(PERF_COUNT_HW_CPU_CYCLES, LinuxPerformanceCounters::Target(&mVal.cpuCycles, true, false)); + } mHas.instructions = mPc->monitor(PERF_COUNT_HW_INSTRUCTIONS, LinuxPerformanceCounters::Target(&mVal.instructions, true, true)); mHas.branchInstructions = mPc->monitor(PERF_COUNT_HW_BRANCH_INSTRUCTIONS, LinuxPerformanceCounters::Target(&mVal.branchInstructions, true, false)); mHas.branchMisses = mPc->monitor(PERF_COUNT_HW_BRANCH_MISSES, LinuxPerformanceCounters::Target(&mVal.branchMisses, true, false)); // mHas.branchMisses = false; + // SW events + mHas.pageFaults = mPc->monitor(PERF_COUNT_SW_PAGE_FAULTS, LinuxPerformanceCounters::Target(&mVal.pageFaults, true, false)); + mHas.contextSwitches = + mPc->monitor(PERF_COUNT_SW_CONTEXT_SWITCHES, LinuxPerformanceCounters::Target(&mVal.contextSwitches, true, false)); + mPc->start(); mPc->calibrate([] { auto before = ankerl::nanobench::Clock::now(); @@ -2789,7 +2803,7 @@ void StreamStateRestorer::restore() { Number::Number(int width, int precision, int64_t value) : mWidth(width) , mPrecision(precision) - , mValue(static_cast(value)) {} + , mValue(d(value)) {} Number::Number(int width, int precision, double value) : mWidth(width) @@ -2823,7 +2837,7 @@ std::ostream& operator<<(std::ostream& os, Number const& n) { return n.write(os); } -MarkDownColumn::MarkDownColumn(int w, int prec, std::string tit, std::string suff, double val) +MarkDownColumn::MarkDownColumn(int w, int prec, std::string tit, std::string suff, double val) noexcept : mWidth(w) , mPrecision(prec) , mTitle(std::move(tit)) @@ -2884,14 +2898,14 @@ std::ostream& operator<<(std::ostream& os, MarkDownCode const& mdCode) { Config::Config() = default; Config::~Config() = default; Config& Config::operator=(Config const&) = default; -Config& Config::operator=(Config&&) noexcept = default; +Config& Config::operator=(Config&&) noexcept(ANKERL_NANOBENCH(NOEXCEPT_STRING_MOVE)) = default; Config::Config(Config const&) = default; Config::Config(Config&&) noexcept = default; // provide implementation here so it's only generated once Result::~Result() = default; Result& Result::operator=(Result const&) = default; -Result& Result::operator=(Result&&) noexcept = default; +Result& Result::operator=(Result&&) noexcept(ANKERL_NANOBENCH(NOEXCEPT_STRING_MOVE)) = default; Result::Result(Result const&) = default; Result::Result(Result&&) noexcept = default; @@ -2992,7 +3006,7 @@ double Result::medianAbsolutePercentError(Measure m) const { auto data = mNameToMeasurements[detail::u(m)]; // calculates MdAPE which is the median of percentage error - // see https://www.spiderfinancial.com/support/documentation/numxl/reference-manual/forecasting-performance/mdape + // see https://support.numxl.com/hc/en-us/articles/115001223503-MdAPE-Median-Absolute-Percentage-Error auto med = calcMedian(data); // transform the data to absolute error @@ -3106,7 +3120,7 @@ Bench::Bench() { } Bench::Bench(Bench&&) noexcept = default; -Bench& Bench::operator=(Bench&&) noexcept = default; +Bench& Bench::operator=(Bench&&) noexcept(ANKERL_NANOBENCH(NOEXCEPT_STRING_MOVE)) = default; Bench::Bench(Bench const&) = default; Bench& Bench::operator=(Bench const&) = default; Bench::~Bench() noexcept = default; @@ -3423,7 +3437,7 @@ BigO::BigO(std::string bigOName, RangeMeasure const& rangeMeasure) sumMeasure += rm.second; } - auto n = static_cast(rangeMeasure.size()); + auto n = detail::d(rangeMeasure.size()); auto mean = sumMeasure / n; mNormalizedRootMeanSquare = std::sqrt(err / n) / mean; }