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:

  544709763e/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
This commit is contained in:
MarcoFalke 2020-04-16 16:37:22 -04:00
commit 969ee85494
No known key found for this signature in database
GPG Key ID: CE2B75697E69A548
3 changed files with 29 additions and 28 deletions

View File

@ -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);
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=<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);
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;

View File

@ -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
{

View File

@ -192,6 +192,7 @@ protected:
public:
ArgsManager();
~ArgsManager();
/**
* Select the network in use