From fa2bc4141df59f2e38fef863723b433250295d20 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 15 Apr 2020 18:53:08 -0400 Subject: [PATCH 1/4] tools: Add unused argsman to bench_bitcoin --- src/bench/bench_bitcoin.cpp | 5 +++-- src/util/settings.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index 2d44264e53d..eeb423c1d29 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -17,7 +17,7 @@ static const char* DEFAULT_PLOT_PLOTLYURL = "https://cdn.plot.ly/plotly-latest.m static const int64_t DEFAULT_PLOT_WIDTH = 1024; static const int64_t DEFAULT_PLOT_HEIGHT = 768; -static void SetupBenchArgs() +static void SetupBenchArgs(ArgsManager& argsman) { SetupHelpOptions(gArgs); @@ -33,7 +33,8 @@ static void SetupBenchArgs() int main(int argc, char** argv) { - SetupBenchArgs(); + ArgsManager argsman; + SetupBenchArgs(argsman); std::string error; if (!gArgs.ParseParameters(argc, argv, error)) { tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error); diff --git a/src/util/settings.h b/src/util/settings.h index 1d03639fa26..bbb6abe2c09 100644 --- a/src/util/settings.h +++ b/src/util/settings.h @@ -9,7 +9,7 @@ #include #include -class UniValue; +#include // For util::SettingsValue = UniValue namespace util { From fa46aebeb1fc419b227524eab8352d9a7fc1f981 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 15 Apr 2020 19:06:59 -0400 Subject: [PATCH 2/4] scripted-diff: Replace gArgs with local argsman in bench -BEGIN VERIFY SCRIPT- sed -i -e 's/gArgs/argsman/g' src/bench/bench_bitcoin.cpp -END VERIFY SCRIPT- --- src/bench/bench_bitcoin.cpp | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index eeb423c1d29..e1983bf9949 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -19,16 +19,16 @@ static const int64_t DEFAULT_PLOT_HEIGHT = 768; static void SetupBenchArgs(ArgsManager& argsman) { - SetupHelpOptions(gArgs); + SetupHelpOptions(argsman); - gArgs.AddArg("-list", "List benchmarks without executing them. Can be combined with -scaling and -filter", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-evals=", strprintf("Number of measurement evaluations to perform. (default: %u)", DEFAULT_BENCH_EVALUATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-filter=", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-scaling=", strprintf("Scaling factor for benchmark's runtime (default: %u)", DEFAULT_BENCH_SCALING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-printer=(console|plot)", strprintf("Choose printer format. console: print data to console. plot: Print results as HTML graph (default: %s)", DEFAULT_BENCH_PRINTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-plot-plotlyurl=", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-plot-width=", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-plot-height=", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-list", "List benchmarks without executing them. Can be combined with -scaling and -filter", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-evals=", strprintf("Number of measurement evaluations to perform. (default: %u)", DEFAULT_BENCH_EVALUATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-filter=", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-scaling=", strprintf("Scaling factor for benchmark's runtime (default: %u)", DEFAULT_BENCH_SCALING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-printer=(console|plot)", strprintf("Choose printer format. console: print data to console. plot: Print results as HTML graph (default: %s)", DEFAULT_BENCH_PRINTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-plot-plotlyurl=", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-plot-width=", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-plot-height=", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); } int main(int argc, char** argv) @@ -36,21 +36,21 @@ int main(int argc, char** argv) ArgsManager argsman; SetupBenchArgs(argsman); std::string error; - if (!gArgs.ParseParameters(argc, argv, error)) { + if (!argsman.ParseParameters(argc, argv, error)) { tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error); return EXIT_FAILURE; } - if (HelpRequested(gArgs)) { - std::cout << gArgs.GetHelpMessage(); + if (HelpRequested(argsman)) { + std::cout << argsman.GetHelpMessage(); return EXIT_SUCCESS; } - int64_t evaluations = gArgs.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS); - std::string regex_filter = gArgs.GetArg("-filter", DEFAULT_BENCH_FILTER); - std::string scaling_str = gArgs.GetArg("-scaling", DEFAULT_BENCH_SCALING); - bool is_list_only = gArgs.GetBoolArg("-list", false); + int64_t evaluations = argsman.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS); + std::string regex_filter = argsman.GetArg("-filter", DEFAULT_BENCH_FILTER); + std::string scaling_str = argsman.GetArg("-scaling", DEFAULT_BENCH_SCALING); + bool is_list_only = argsman.GetBoolArg("-list", false); if (evaluations == 0) { return EXIT_SUCCESS; @@ -66,15 +66,15 @@ int main(int argc, char** argv) } std::unique_ptr printer = MakeUnique(); - std::string printer_arg = gArgs.GetArg("-printer", DEFAULT_BENCH_PRINTER); + std::string printer_arg = argsman.GetArg("-printer", DEFAULT_BENCH_PRINTER); if ("plot" == printer_arg) { printer.reset(new benchmark::PlotlyPrinter( - gArgs.GetArg("-plot-plotlyurl", DEFAULT_PLOT_PLOTLYURL), - gArgs.GetArg("-plot-width", DEFAULT_PLOT_WIDTH), - gArgs.GetArg("-plot-height", DEFAULT_PLOT_HEIGHT))); + argsman.GetArg("-plot-plotlyurl", DEFAULT_PLOT_PLOTLYURL), + argsman.GetArg("-plot-width", DEFAULT_PLOT_WIDTH), + argsman.GetArg("-plot-height", DEFAULT_PLOT_HEIGHT))); } - gArgs.ClearArgs(); // gArgs no longer needed. Clear it here to avoid interactions with the testing setup in the benches + argsman.ClearArgs(); // argsman no longer needed. Clear it here to avoid interactions with the testing setup in the benches benchmark::BenchRunner::RunAll(*printer, evaluations, scaling_factor, regex_filter, is_list_only); From fae00a77e2589cc784650e3e60e1b99c22ca8a7b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 15 Apr 2020 19:02:22 -0400 Subject: [PATCH 3/4] bench: Remove unused argsman.ClearArgs --- src/bench/bench_bitcoin.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index e1983bf9949..9b81380a9ba 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -74,8 +74,6 @@ int main(int argc, char** argv) argsman.GetArg("-plot-height", DEFAULT_PLOT_HEIGHT))); } - argsman.ClearArgs(); // argsman no longer needed. Clear it here to avoid interactions with the testing setup in the benches - benchmark::BenchRunner::RunAll(*printer, evaluations, scaling_factor, regex_filter, is_list_only); return EXIT_SUCCESS; From faf989f93695d29099f6e152d5a2e117cd304183 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 16 Apr 2020 12:26:01 -0400 Subject: [PATCH 4/4] util: Document why ArgsManager (con/de)structor is not inline --- src/util/settings.h | 2 +- src/util/system.cpp | 9 +++++---- src/util/system.h | 1 + 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/util/settings.h b/src/util/settings.h index bbb6abe2c09..1d03639fa26 100644 --- a/src/util/settings.h +++ b/src/util/settings.h @@ -9,7 +9,7 @@ #include #include -#include // For util::SettingsValue = UniValue +class UniValue; namespace util { diff --git a/src/util/system.cpp b/src/util/system.cpp index b0a538b5277..69a7be96dc8 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -226,10 +226,11 @@ static bool CheckValid(const std::string& key, const util::SettingsValue& val, u return true; } -ArgsManager::ArgsManager() -{ - // nothing to do -} +// Define default constructor and destructor that are not inline, so code instantiating this class doesn't need to +// #include class definitions for all members. +// For example, m_settings has an internal dependency on univalue. +ArgsManager::ArgsManager() {} +ArgsManager::~ArgsManager() {} const std::set ArgsManager::GetUnsuitableSectionOnlyArgs() const { diff --git a/src/util/system.h b/src/util/system.h index 3138522b5c8..96f51e6074a 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -192,6 +192,7 @@ protected: public: ArgsManager(); + ~ArgsManager(); /** * Select the network in use