Merge pull request #4378 from randombit/jack/speed-cleanup

Cleanups after refactoring of speed cli

src/cli/perf.cpp

@@ -38,4 +38,14 @@ std::unique_ptr<PerfTest> PerfTest::get(const std::string& name) {
  return PerfTest::get_sym(name);
 }
 
+std::string PerfTest::format_name(const std::string& alg, const std::string& param) const {
+ if(param.empty()) {
+ return alg;
+ }
+ if(param.starts_with(alg)) {
+ return param;
+ }
+ return Botan::fmt("{}-{}", alg, param);
+}
+
 } // namespace Botan_CLI

src/cli/perf.h

@@ -18,27 +18,55 @@
 
 namespace Botan_CLI {
 
-class PerfConfig {
+class PerfConfig final {
  public:
- virtual ~PerfConfig() = default;
+ PerfConfig(std::function<void(const Botan::Timer&)> record_result,
+ size_t clock_speed,
+ double clock_cycle_ratio,
+ std::chrono::milliseconds runtime,
+ const std::vector<std::string>& ecc_groups,
+ const std::vector<size_t>& buffer_sizes,
+ std::ostream& error_output,
+ Botan::RandomNumberGenerator& rng) :
+ m_record_result(std::move(record_result)),
+ m_clock_speed(clock_speed),
+ m_clock_cycle_ratio(clock_cycle_ratio),
+ m_runtime(runtime),
+ m_ecc_groups(ecc_groups),
+ m_buffer_sizes(buffer_sizes),
+ m_error_output(error_output),
+ m_rng(rng) {}
+
+ const std::vector<size_t>& buffer_sizes() const { return m_buffer_sizes; }
+
+ const std::vector<std::string>& ecc_groups() const { return m_ecc_groups; }
+
+ std::chrono::milliseconds runtime() const { return m_runtime; }
+
+ std::ostream& error_output() const { return m_error_output; }
+
+ Botan::RandomNumberGenerator& rng() const { return m_rng; }
+
+ void record_result(const Botan::Timer& timer) const { m_record_result(timer); }
+
+ std::unique_ptr<Botan::Timer> make_timer(const std::string& alg,
+ uint64_t event_mult = 1,
+ const std::string& what = "",
+ const std::string& provider = "",
+ size_t buf_size = 0) const {
+ return std::make_unique<Botan::Timer>(
+ alg, provider, what, event_mult, buf_size, m_clock_cycle_ratio, m_clock_speed);
+ }
 
- virtual std::chrono::milliseconds runtime() const = 0;
-
- virtual const std::vector<std::string>& ecc_groups() const = 0;
-
- virtual const std::vector<size_t>& buffer_sizes() const = 0;
-
- virtual std::ostream& error_output() const = 0;
-
- virtual Botan::RandomNumberGenerator& rng() const = 0;
-
- virtual void record_result(const Botan::Timer& timer) const = 0;
-
- virtual std::unique_ptr<Botan::Timer> make_timer(const std::string& alg,
- uint64_t event_mult = 1,
- const std::string& what = "",
- const std::string& provider = "",
- size_t buf_size = 0) const = 0;
+ private:
+ std::function<void(const Botan::Timer&)> m_record_result;
+ size_t m_clock_speed = 0;
+ double m_clock_cycle_ratio = 0.0;
+ std::chrono::milliseconds m_runtime;
+ std::vector<std::string> m_ecc_groups;
+ std::vector<size_t> m_buffer_sizes;
+ std::ostream& m_error_output;
+ Botan::RandomNumberGenerator& m_rng;
 };
 
 class PerfTest {
@@ -50,6 +78,8 @@ class PerfTest {
 
  virtual void go(const PerfConfig& config) = 0;
 
+ virtual std::string format_name(const std::string& alg, const std::string& param) const;
+
  typedef std::function<std::unique_ptr<PerfTest>()> pt_maker_fn;
 
  class Registration final {

src/cli/perf_pk_enc.cpp

@@ -24,10 +24,6 @@ class PerfTest_PKEnc : public PerfTest {
  return {""};
  }
 
- virtual std::string format_name(const std::string& alg, const std::string& param) const {
- return param.empty() ? alg : Botan::fmt("{}-{}", alg, param);
- }
-
  void go(const PerfConfig& config) override {
  const std::string alg = this->algo();
 

src/cli/perf_pk_ka.cpp

@@ -24,10 +24,6 @@ class PerfTest_PKKa : public PerfTest {
  return {""};
  }
 
- virtual std::string format_name(const std::string& alg, const std::string& param) const {
- return param.empty() ? alg : Botan::fmt("{}-{}", alg, param);
- }
-
  void go(const PerfConfig& config) override {
  const std::string alg = this->algo();
 

src/cli/perf_pk_kem.cpp

@@ -24,10 +24,6 @@ class PerfTest_PK_KEM : public PerfTest {
  return {""};
  }
 
- virtual std::string format_name(const std::string& alg, const std::string& param) const {
- return param.empty() ? alg : Botan::fmt("{}-{}", alg, param);
- }
-
  void go(const PerfConfig& config) override {
  const std::string alg = this->algo();
 
@@ -97,9 +93,6 @@ class PerfTest_Kyber final : public PerfTest_PK_KEM {
  "Kyber-768-90s-r3",
  "Kyber-1024-r3",
  "Kyber-1024-90s-r3",
- "ML-KEM-512",
- "ML-KEM-768",
- "ML-KEM-1024",
  };
  }
 };

src/cli/perf_pk_sig.cpp

@@ -26,10 +26,6 @@ class PerfTest_PKSig : public PerfTest {
  return {""};
  }
 
- virtual std::string format_name(const std::string& alg, const std::string& param) const {
- return param.empty() ? alg : Botan::fmt("{}-{}", alg, param);
- }
-
  void go(const PerfConfig& config) override {
  const std::string alg = this->algo();
  const std::string padding = this->hash();
@@ -258,11 +254,6 @@ class PerfTest_XMSS final : public PerfTest_PKSig {
 
  std::string hash() const override { return ""; }
 
- std::string format_name(const std::string& alg, const std::string& param) const override {
- BOTAN_UNUSED(alg);
- return param; // Param already has XMSS in the name...
- }
-
  std::vector<std::string> keygen_params(const PerfConfig& config) const override {
  BOTAN_UNUSED(config);
 
@@ -321,8 +312,7 @@ class PerfTest_SPHINCSp final : public PerfTest_PKSig {
  std::string hash() const override { return ""; }
 
  std::string format_name(const std::string& alg, const std::string& param) const override {
- BOTAN_UNUSED(alg);
- return param; // Param already has algo in the string
+ return alg + param.substr(11);
  }
 
  std::vector<std::string> keygen_params(const PerfConfig& config) const override {
@@ -355,11 +345,6 @@ class PerfTest_SLH_DSA final : public PerfTest_PKSig {
 
  std::string hash() const override { return ""; }
 
- std::string format_name(const std::string& alg, const std::string& param) const override {
- BOTAN_UNUSED(alg);
- return param; // Param already has algo in the string
- }
-
  std::vector<std::string> keygen_params(const PerfConfig& config) const override {
  BOTAN_UNUSED(config);
 
@@ -390,11 +375,6 @@ class PerfTest_Dilithium final : public PerfTest_PKSig {
 
  std::string hash() const override { return ""; }
 
- std::string format_name(const std::string& alg, const std::string& param) const override {
- BOTAN_UNUSED(alg);
- return param; // Param already has algo in the string
- }
-
  std::vector<std::string> keygen_params(const PerfConfig& config) const override {
  BOTAN_UNUSED(config);
 
@@ -421,11 +401,6 @@ class PerfTest_ML_DSA final : public PerfTest_PKSig {
 
  std::string hash() const override { return ""; }
 
- std::string format_name(const std::string& alg, const std::string& param) const override {
- BOTAN_UNUSED(alg);
- return param; // Param already has algo in the string
- }
-
  std::vector<std::string> keygen_params(const PerfConfig& config) const override {
  BOTAN_UNUSED(config);
 

src/cli/speed.cpp

@@ -283,10 +283,9 @@ class Speed final : public Command {
  "Ed448",
  "X25519",
  "X448",
- "Kyber",
  "ML-KEM",
+ "ML-DSA",
  "SLH-DSA",
- "SPHINCS+",
  "FrodoKEM",
  "HSS-LMS",
  };
@@ -302,26 +301,27 @@ class Speed final : public Command {
  std::vector<std::string> ecc_groups = Command::split_on(get_arg("ecc-groups"), ',');
  const std::string format = get_arg("format");
  const std::string clock_ratio = get_arg("cpu-clock-ratio");
- m_clock_speed = get_arg_sz("cpu-clock-speed");
 
- m_clock_cycle_ratio = std::strtod(clock_ratio.c_str(), nullptr);
+ const size_t clock_speed = get_arg_sz("cpu-clock-speed");
+
+ double clock_cycle_ratio = std::strtod(clock_ratio.c_str(), nullptr);
 
  /*
  * This argument is intended to be the ratio between the cycle counter
  * and the actual machine cycles. It is extremely unlikely that there is
  * any machine where the cycle counter increments faster than the actual
  * clock.
  */
- if(m_clock_cycle_ratio < 0.0 || m_clock_cycle_ratio > 1.0) {
+ if(clock_cycle_ratio < 0.0 || clock_cycle_ratio > 1.0) {
  throw CLI_Usage_Error("Unlikely CPU clock ratio of " + clock_ratio);
  }
 
- m_clock_cycle_ratio = 1.0 / m_clock_cycle_ratio;
+ clock_cycle_ratio = 1.0 / clock_cycle_ratio;
 
- if(m_clock_speed != 0 && Botan::OS::get_cpu_cycle_counter() != 0) {
+ if(clock_speed != 0 && Botan::OS::get_cpu_cycle_counter() != 0) {
  error_output() << "The --cpu-clock-speed option is only intended to be used on "
  "platforms without access to a cycle counter.\n"
- "Expected incorrect results\n\n";
+ "Expect incorrect results\n\n";
  }
 
  if(format == "table") {
@@ -366,46 +366,16 @@ class Speed final : public Command {
  algos = default_benchmark_list();
  }
 
- class PerfConfig_Cli final : public PerfConfig {
- public:
- PerfConfig_Cli(std::chrono::milliseconds runtime,
- const std::vector<std::string>& ecc_groups,
- const std::vector<size_t>& buffer_sizes,
- Speed* speed) :
- m_runtime(runtime), m_ecc_groups(ecc_groups), m_buffer_sizes(buffer_sizes), m_speed(speed) {}
-
- const std::vector<size_t>& buffer_sizes() const override { return m_buffer_sizes; }
-
- const std::vector<std::string>& ecc_groups() const override { return m_ecc_groups; }
-
- std::chrono::milliseconds runtime() const override { return m_runtime; }
-
- std::ostream& error_output() const override { return m_speed->error_output(); }
-
- Botan::RandomNumberGenerator& rng() const override { return m_speed->rng(); }
-
- void record_result(const Botan::Timer& timer) const override { m_speed->record_result(timer); }
-
- std::unique_ptr<Botan::Timer> make_timer(const std::string& alg,
- uint64_t event_mult,
- const std::string& what,
- const std::string& provider,
- size_t buf_size) const override {
- return m_speed->make_timer(alg, event_mult, what, provider, buf_size);
- }
-
- private:
- std::chrono::milliseconds m_runtime;
- std::vector<std::string> m_ecc_groups;
- std::vector<size_t> m_buffer_sizes;
- Speed* m_speed;
- };
-
- PerfConfig_Cli perf_config(msec, ecc_groups, buf_sizes, this);
+ PerfConfig perf_config([&](const Botan::Timer& t) { this->record_result(t); },
+ clock_speed,
+ clock_cycle_ratio,
+ msec,
+ ecc_groups,
+ buf_sizes,
+ this->error_output(),
+ this->rng());
 
  for(const auto& algo : algos) {
- using namespace std::placeholders;
-
  if(auto perf = PerfTest::get(algo)) {
  perf->go(perf_config);
  } else if(verbose() || !using_defaults) {
@@ -420,7 +390,7 @@ class Speed final : public Command {
  output() << m_summary->print() << "\n";
  }
 
- if(verbose() && m_clock_speed == 0 && m_cycles_consumed > 0 && m_ns_taken > 0) {
+ if(verbose() && clock_speed == 0 && m_cycles_consumed > 0 && m_ns_taken > 0) {
  const double seconds = static_cast<double>(m_ns_taken) / 1000000000;
  const double Hz = static_cast<double>(m_cycles_consumed) / seconds;
  const double MHz = Hz / 1000000;
@@ -429,8 +399,6 @@ class Speed final : public Command {
  }
 
  private:
- size_t m_clock_speed = 0;
- double m_clock_cycle_ratio = 0.0;
  uint64_t m_cycles_consumed = 0;
  uint64_t m_ns_taken = 0;
  std::unique_ptr<Summary> m_summary;
@@ -448,16 +416,6 @@ class Speed final : public Command {
  }
  }
  }
-
- void record_result(const std::unique_ptr<Timer>& t) { record_result(*t); }
-
- std::unique_ptr<Timer> make_timer(const std::string& name,
- uint64_t event_mult = 1,
- const std::string& what = "",
- const std::string& provider = "",
- size_t buf_size = 0) {
- return std::make_unique<Timer>(name, provider, what, event_mult, buf_size, m_clock_cycle_ratio, m_clock_speed);
- }
 };
 
 BOTAN_REGISTER_COMMAND("speed", Speed);