From 5025990aed4299420081638720c853bc425c9003 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 2 Feb 2023 10:24:52 +0000 Subject: [PATCH 1/8] Merge bitcoin/bitcoin#27005: util: Use steady clock for logging timer fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 Use steady clock for logging timer (MarcoFalke) Pull request description: The logging timer has many issues: * The underlying clock is mockable, meaning that benchmarks are useless when mocktime was set at the beginning or end of the benchmark. * The underlying clock is not monotonic, meaning that benchmarks are useless when the system time was changed during the benchmark. Fix all issues in this patch. ACKs for top commit: stickies-v: Approach ACK fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 john-moffett: ACK fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 Tree-SHA512: bec8da0f338ed4611e1807937575e1b2afda25139d88015b1c29fa7d13946fbfbc4ee589b576c0508d505df5e5fafafcbc07d63ce4bab4b01475260d9d5d2107 --- src/logging/timer.h | 26 ++++++++++++++------------ src/test/logging_tests.cpp | 15 ++------------- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/logging/timer.h b/src/logging/timer.h index d954e4630178..f4114597cba7 100644 --- a/src/logging/timer.h +++ b/src/logging/timer.h @@ -12,6 +12,7 @@ #include #include +#include #include @@ -28,14 +29,14 @@ class Timer std::string prefix, std::string end_msg, BCLog::LogFlags log_category = BCLog::LogFlags::ALL, - bool msg_on_completion = true) : - m_prefix(std::move(prefix)), - m_title(std::move(end_msg)), - m_log_category(log_category), - m_message_on_completion(msg_on_completion) + bool msg_on_completion = true) + : m_prefix(std::move(prefix)), + m_title(std::move(end_msg)), + m_log_category(log_category), + m_message_on_completion(msg_on_completion) { this->Log(strprintf("%s started", m_title)); - m_start_t = GetTime(); + m_start_t = std::chrono::steady_clock::now(); } ~Timer() @@ -60,24 +61,25 @@ class Timer std::string LogMsg(const std::string& msg) { - const auto end_time = GetTime() - m_start_t; - if (m_start_t.count() <= 0) { + const auto end_time{std::chrono::steady_clock::now()}; + if (!m_start_t) { return strprintf("%s: %s", m_prefix, msg); } + const auto duration{end_time - *m_start_t}; if constexpr (std::is_same::value) { - return strprintf("%s: %s (%iμs)", m_prefix, msg, end_time.count()); + return strprintf("%s: %s (%iμs)", m_prefix, msg, Ticks(duration)); } else if constexpr (std::is_same::value) { - return strprintf("%s: %s (%.2fms)", m_prefix, msg, end_time.count() * 0.001); + return strprintf("%s: %s (%.2fms)", m_prefix, msg, Ticks(duration)); } else if constexpr (std::is_same::value) { - return strprintf("%s: %s (%.2fs)", m_prefix, msg, end_time.count() * 0.000001); + return strprintf("%s: %s (%.2fs)", m_prefix, msg, Ticks(duration)); } else { static_assert(ALWAYS_FALSE, "Error: unexpected time type"); } } private: - std::chrono::microseconds m_start_t{}; + std::optional m_start_t{}; //! Log prefix; usually the name of the function this was created in. const std::string m_prefix; diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index afaf78f00879..b07574c0d13a 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -75,20 +75,9 @@ struct LogSetup : public BasicTestingSetup { BOOST_AUTO_TEST_CASE(logging_timer) { - SetMockTime(1); auto micro_timer = BCLog::Timer("tests", "end_msg"); - SetMockTime(2); - BOOST_CHECK_EQUAL(micro_timer.LogMsg("test micros"), "tests: test micros (1000000μs)"); - - SetMockTime(1); - auto ms_timer = BCLog::Timer("tests", "end_msg"); - SetMockTime(2); - BOOST_CHECK_EQUAL(ms_timer.LogMsg("test ms"), "tests: test ms (1000.00ms)"); - - SetMockTime(1); - auto sec_timer = BCLog::Timer("tests", "end_msg"); - SetMockTime(2); - BOOST_CHECK_EQUAL(sec_timer.LogMsg("test secs"), "tests: test secs (1.00s)"); + const std::string_view result_prefix{"tests: msg ("}; + BOOST_CHECK_EQUAL(micro_timer.LogMsg("msg").substr(0, result_prefix.size()), result_prefix); } BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) From e822355646adb45bab53441a4186f49f59bd93fb Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 13 Oct 2023 10:16:16 +0200 Subject: [PATCH 2/8] Merge bitcoin/bitcoin#28644: test: Fuzz merge with -use_value_profile=0 for now faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 test: Fuzz merge with -use_value_profile=0 for now (MarcoFalke) Pull request description: Seems odd that this has to be done, but for now there are (unknown) size limits on the qa-assets repo. Also, a larger size means that cloning and iterating over the files takes a longer time. Not sure how to measure the net impact of this, but with some backups reverting this commit, it can be limited on the downside? ACKs for top commit: dergoegge: ACK faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 Tree-SHA512: 9f8b3f4526f60e4ff6fca97859a725d145a8339c216bd15c92fad7e53f84308745fee47727527de459c0245ef9d474a9dc836fee599ab2b556b519bd900b9a33 --- test/fuzz/test_runner.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 399d9209e280..f5dd73565078 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2019-2020 The Bitcoin Core developers +# Copyright (c) 2019-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Run fuzz test targets. @@ -249,7 +249,11 @@ def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, fuzz_bin, merge_dir): # [0] https://github.com/bitcoin-core/qa-assets/issues/130#issuecomment-1761760866 '-shuffle=0', '-prefer_small=1', - '-use_value_profile=1', # Also done by oss-fuzz https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487 + '-use_value_profile=0', + # use_value_profile is enabled by oss-fuzz [0], but disabled for + # now to avoid bloating the qa-assets git repository [1]. + # [0] https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487 + # [1] https://github.com/bitcoin-core/qa-assets/issues/130#issuecomment-1749075891 os.path.join(corpus, t), os.path.join(merge_dir, t), ] From 2b95fb23441f60e07129e2b83dd59b889ded8ed6 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 17 Feb 2023 17:20:48 -0500 Subject: [PATCH 3/8] Merge bitcoin/bitcoin#26940: test: create random and coins utils, add amount helper, dedupe add_coin 4275195606e6f42466d9a8ef766b3035833df4d5 De-duplicate add_coin methods to a test util helper (Jon Atack) 9d92c3d7f42c18939a9a6aa1ee185f1c958360a0 Create InsecureRandMoneyAmount() test util helper (Jon Atack) 81f5ade2a324167c03c5ce765a26bd42ed652723 Move random test util code from setup_common to random (Jon Atack) Pull request description: - Move random test utilities from `setup_common` to a new `random` file, as many tests don't use this code. - Create a helper to generate semi-random CAmounts up to `MONEY_RANGE` rather than only uint32, and use the helper in the unit tests. - De-duplicate a shared `add_coin` method by extracting it to a `coins` test utility. ACKs for top commit: pinheadmz: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 achow101: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 john-moffett: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 Tree-SHA512: 3ed974251149c7417f935ef2f8865aa0dcc33b281b47522b0f96f1979dff94bb8527957f098fe4d210f40d715c00f29512f2ffe189097102229023b7284a3a27 Merge bitcoin/bitcoin#26940: test: create random and coins utils, add amount helper, dedupe add_coin 4275195606e6f42466d9a8ef766b3035833df4d5 De-duplicate add_coin methods to a test util helper (Jon Atack) 9d92c3d7f42c18939a9a6aa1ee185f1c958360a0 Create InsecureRandMoneyAmount() test util helper (Jon Atack) 81f5ade2a324167c03c5ce765a26bd42ed652723 Move random test util code from setup_common to random (Jon Atack) Pull request description: - Move random test utilities from `setup_common` to a new `random` file, as many tests don't use this code. - Create a helper to generate semi-random CAmounts up to `MONEY_RANGE` rather than only uint32, and use the helper in the unit tests. - De-duplicate a shared `add_coin` method by extracting it to a `coins` test utility. ACKs for top commit: pinheadmz: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 achow101: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 john-moffett: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 Tree-SHA512: 3ed974251149c7417f935ef2f8865aa0dcc33b281b47522b0f96f1979dff94bb8527957f098fe4d210f40d715c00f29512f2ffe189097102229023b7284a3a27 Merge bitcoin/bitcoin#26940: test: create random and coins utils, add amount helper, dedupe add_coin 4275195606e6f42466d9a8ef766b3035833df4d5 De-duplicate add_coin methods to a test util helper (Jon Atack) 9d92c3d7f42c18939a9a6aa1ee185f1c958360a0 Create InsecureRandMoneyAmount() test util helper (Jon Atack) 81f5ade2a324167c03c5ce765a26bd42ed652723 Move random test util code from setup_common to random (Jon Atack) Pull request description: - Move random test utilities from `setup_common` to a new `random` file, as many tests don't use this code. - Create a helper to generate semi-random CAmounts up to `MONEY_RANGE` rather than only uint32, and use the helper in the unit tests. - De-duplicate a shared `add_coin` method by extracting it to a `coins` test utility. ACKs for top commit: pinheadmz: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 achow101: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 john-moffett: ACK 4275195606e6f42466d9a8ef766b3035833df4d5 Tree-SHA512: 3ed974251149c7417f935ef2f8865aa0dcc33b281b47522b0f96f1979dff94bb8527957f098fe4d210f40d715c00f29512f2ffe189097102229023b7284a3a27 --- src/Makefile.test_util.include | 3 ++ src/test/base58_tests.cpp | 1 + src/test/bip324_tests.cpp | 1 + src/test/blockencodings_tests.cpp | 1 + src/test/bloom_tests.cpp | 1 + src/test/checkqueue_tests.cpp | 1 + src/test/coins_tests.cpp | 3 +- src/test/crypto_tests.cpp | 1 + src/test/cuckoocache_tests.cpp | 1 + src/test/dbwrapper_tests.cpp | 1 + src/test/hash_tests.cpp | 1 + src/test/key_tests.cpp | 1 + src/test/merkle_tests.cpp | 1 + src/test/miner_tests.cpp | 1 + src/test/minisketch_tests.cpp | 1 + src/test/net_tests.cpp | 1 + src/test/orphanage_tests.cpp | 1 + src/test/pmt_tests.cpp | 1 + src/test/pool_tests.cpp | 1 + src/test/pow_tests.cpp | 1 + src/test/prevector_tests.cpp | 2 + src/test/script_tests.cpp | 1 + src/test/serfloat_tests.cpp | 1 + src/test/sighash_tests.cpp | 3 +- src/test/skiplist_tests.cpp | 1 + src/test/streams_tests.cpp | 1 + src/test/transaction_tests.cpp | 1 + src/test/txpackage_tests.cpp | 1 + src/test/util/blockfilter.cpp | 1 - src/test/util/coins.cpp | 27 +++++++++++ src/test/util/coins.h | 19 ++++++++ src/test/util/llmq_tests.h | 1 + src/test/util/random.h | 45 +++++++++++++++++++ src/test/util/setup_common.h | 6 --- src/test/util_tests.cpp | 1 + src/test/validation_block_tests.cpp | 1 + src/test/validation_chainstate_tests.cpp | 18 ++------ .../validation_chainstatemanager_tests.cpp | 1 + src/test/validation_flush_tests.cpp | 27 ++++------- src/test/versionbits_tests.cpp | 1 + src/wallet/test/wallet_crypto_tests.cpp | 1 + 41 files changed, 141 insertions(+), 43 deletions(-) create mode 100644 src/test/util/coins.cpp create mode 100644 src/test/util/coins.h create mode 100644 src/test/util/random.h diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index 749d3a5f1d62..5eae384ebfdc 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -10,6 +10,7 @@ EXTRA_LIBRARIES += \ TEST_UTIL_H = \ test/util/blockfilter.h \ test/util/chainstate.h \ + test/util/coins.h \ test/util/json.h \ test/util/index.h \ test/util/llmq_tests.h \ @@ -17,6 +18,7 @@ TEST_UTIL_H = \ test/util/mining.h \ test/util/net.h \ test/util/poolresourcetester.h \ + test/util/random.h \ test/util/script.h \ test/util/setup_common.h \ test/util/str.h \ @@ -31,6 +33,7 @@ libtest_util_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libtest_util_a_SOURCES = \ test/util/blockfilter.cpp \ test/util/index.cpp \ + test/util/coins.cpp \ test/util/json.cpp \ test/util/logging.cpp \ test/util/mining.cpp \ diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 2cd1ae9510d2..af2e87eedf4e 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -7,6 +7,7 @@ #include #include #include // for InsecureRand* +#include #include #include diff --git a/src/test/bip324_tests.cpp b/src/test/bip324_tests.cpp index 7d651d72b38d..5d6bef2ec7eb 100644 --- a/src/test/bip324_tests.cpp +++ b/src/test/bip324_tests.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index c7ed320cf9d2..e5ccccfe2dc3 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 47728ba30a17..9be0e52e1e55 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index a73b4c252642..a338511c9fad 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 793c2d84a179..6a6d030d460c 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -7,6 +7,7 @@ #include