Merge #18662: test: Replace gArgs with local argsman in bench

faf989f93695d29099f6e152d5a2e117cd304183 util: Document why ArgsManager (con/de)structor is not inline (MarcoFalke)
fae00a77e2589cc784650e3e60e1b99c22ca8a7b bench: Remove unused argsman.ClearArgs (MarcoFalke)
fa46aebeb1fc419b227524eab8352d9a7fc1f981 scripted-diff: Replace gArgs with local argsman in bench (MarcoFalke)
fa2bc4141df59f2e38fef863723b433250295d20 tools: Add unused argsman to bench_bitcoin (MarcoFalke)

Pull request description:

  All utilities use the same gArgs global that the node uses. This is generally fine and does not lead to issues unless a bench test is going to spin up a NodeContext via the TestingSetup. In that case the two uses of gArgs conflict and currently it needs to be cleared:

  https://github.com/bitcoin/bitcoin/blob/544709763e1f45148d1926831e07ff03487673ee/src/bench/bench_bitcoin.cpp#L76

  One solution would be to do nothing, because the current code works with that workaround. Another solution would be to not use the same global in all binaries.

ACKs for top commit:
  promag:
    ACK faf989f93695d29099f6e152d5a2e117cd304183.
  ryanofsky:
    Code review ACK faf989f93695d29099f6e152d5a2e117cd304183. Just new commit added restoring forward declaration

Tree-SHA512: 8ee4b28eee294d41c002f801fa844b0c23c919a3061f5109638701db0947b3b0ea28caa7311ae5f126fc660648bbaa0890853e6b06bdc5868692f52ba8c05f66

3 files changed, 30 insertions, 29 deletions

diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp
index 2d44264e53..9b81380a9b 100644
--- a/src/bench/bench_bitcoin.cpp
+++ b/src/bench/bench_bitcoin.cpp
@@ -17,39 +17,40 @@ 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);
-
-    gArgs.AddArg("-list", "List benchmarks without executing them. Can be combined with -scaling and -filter", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-evals=<n>", strprintf("Number of measurement evaluations to perform. (default: %u)", DEFAULT_BENCH_EVALUATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-scaling=<n>", 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=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-plot-height=<x>", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    SetupHelpOptions(argsman);
+
+    argsman.AddArg("-list", "List benchmarks without executing them. Can be combined with -scaling and -filter", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-evals=<n>", strprintf("Number of measurement evaluations to perform. (default: %u)", DEFAULT_BENCH_EVALUATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-filter=<regex>", strprintf("Regular expression filter to select benchmark by name (default: %s)", DEFAULT_BENCH_FILTER), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-scaling=<n>", 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=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-plot-height=<x>", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 }
 
 int main(int argc, char** argv)
 {
-    SetupBenchArgs();
+    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;
@@ -65,16 +66,14 @@ int main(int argc, char** argv)
     }
 
     std::unique_ptr<benchmark::Printer> printer = MakeUnique<benchmark::ConsolePrinter>();
-    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
-
     benchmark::BenchRunner::RunAll(*printer, evaluations, scaling_factor, regex_filter, is_list_only);
 
     return EXIT_SUCCESS;
diff --git a/src/util/system.cpp b/src/util/system.cpp
index b0a538b527..69a7be96dc 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<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
 {
diff --git a/src/util/system.h b/src/util/system.h
index 3138522b5c..96f51e6074 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -192,6 +192,7 @@ protected:
 
 public:
     ArgsManager();
+    ~ArgsManager();
 
     /**
      * Select the network in use