Adress requested changes

include/lo2s/config.hpp

@@ -22,6 +22,7 @@
 #pragma once
 
 #include <chrono>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -51,45 +52,44 @@ struct Config
     Process process;
     std::vector<std::string> command;
     std::string command_line;
-    bool quiet;
-    bool drop_root;
+    bool quiet = false;
+    bool drop_root = false;
     std::string user = "";
     // Optional features
     std::vector<std::string> tracepoint_events;
 #ifdef HAVE_X86_ADAPT
     std::vector<std::string> x86_adapt_knobs;
 #endif
-    bool use_sensors;
+    bool use_sensors = false;
     int cgroup_fd = -1;
     // OTF2
     std::string trace_path;
     // perf
     std::size_t mmap_pages;
-    bool exclude_kernel;
+    bool exclude_kernel = false;
     // Instruction sampling
-    bool sampling;
+    bool sampling = false;
     std::uint64_t sampling_period;
     std::string sampling_event;
-    bool enable_cct;
-    bool suppress_ip;
-    bool disassemble;
+    bool enable_cct = false;
+    bool suppress_ip = false;
+    bool disassemble = false;
     // Interval monitors
     std::chrono::nanoseconds read_interval;
     std::chrono::nanoseconds userspace_read_interval;
     std::chrono::nanoseconds perf_read_interval = std::chrono::nanoseconds(0);
     // Metrics
-    bool metric_use_frequency;
+    bool metric_use_frequency = true;
 
     std::uint64_t metric_count;
     std::uint64_t metric_frequency;
 
     std::string metric_leader;
     std::vector<std::string> group_counters;
     std::vector<std::string> userspace_counters;
-    // time synchronization
-    bool use_clockid;
-    bool use_pebs;
-    clockid_t clockid;
+    // time synchronizatio
+    std::optional<clockid_t> clockid;
+    bool use_pebs = false;
     // x86_energy
     bool use_x86_energy = false;
     // block I/O

include/lo2s/perf/event_attr.hpp

@@ -1,5 +1,5 @@
-/*
- * This file is part of the lo2s software.
+
+/* This file is part of the lo2s software.
  * Linux OTF2 sampling
  *
  * Copyright (c) 2024,
@@ -82,7 +82,7 @@ class EventGuard;
 class EventAttr
 {
 public:
-    EventAttr(const std::string name, perf_type_id type, std::uint64_t config,
+    EventAttr(const std::string& name, perf_type_id type, std::uint64_t config,
               std::uint64_t config1 = 0);
 
     class InvalidEvent : public std::runtime_error
@@ -145,16 +145,16 @@ class EventAttr
         unit_ = unit;
     }
 
-    void set_clockid(int64_t clockid)
+    void set_clockid(std::optional<clockid_t> clockid)
     {
-        if (clockid == -1)
+        if (!clockid.has_value())
         {
             attr_.use_clockid = 0;
         }
         else
         {
             attr_.use_clockid = 1;
-            attr_.clockid = clockid;
+            attr_.clockid = clockid.value();
         }
     }
 
@@ -197,7 +197,7 @@ class EventAttr
         }
     }
 
-    bool get_flag(const EventFlag flag)
+    bool get_flag(EventFlag flag)
     {
         switch (flag)
         {
@@ -305,9 +305,9 @@ class EventAttr
 class SimpleEventAttr : public EventAttr
 {
 public:
-    SimpleEventAttr(const std::string name, perf_type_id type, std::uint64_t config,
+    SimpleEventAttr(const std::string& name, perf_type_id type, std::uint64_t config,
                     std::uint64_t config1 = 0);
-    static SimpleEventAttr raw(std::string name);
+    static SimpleEventAttr raw(const std::string& name);
 };
 
 #ifndef USE_HW_BREAKPOINT_COMPAT

include/lo2s/perf/event_composer.hpp

@@ -46,7 +46,7 @@ class EventComposer
 
     EventAttr create_time_event(uint64_t local_time);
     EventAttr create_sampling_event();
-    perf::tracepoint::TracepointEventAttr create_tracepoint_event(std::string name);
+    perf::tracepoint::TracepointEventAttr create_tracepoint_event(const std::string& name);
     std::vector<perf::tracepoint::TracepointEventAttr> get_tracepoints();
 
 private:

include/lo2s/perf/event_resolver.hpp

@@ -43,7 +43,7 @@ class EventResolver
     std::vector<EventAttr> get_predefined_events();
     std::vector<SysfsEventAttr> get_pmu_events();
 
-    EventAttr get_metric_leader(std::string metric_leader);
+    EventAttr get_metric_leader(const std::string& metric_leader);
 
     std::vector<std::string> get_tracepoint_event_names();
 

src/config.cpp

@@ -88,15 +88,27 @@ static inline void print_availability(std::ostream& os, const std::string& descr
             if (ev.supported_cpus() != Topology::instance().cpus())
             {
                 const auto& cpus = ev.supported_cpus();
-                if (cpus.size() == 1)
+                std::vector<int> cpus_int;
+                std::transform(cpus.begin(), cpus.end(), std::back_inserter(cpus_int),
+                               [](Cpu cpu) { return cpu.as_int(); });
+
+                if (cpus_int.size() == 1)
                 {
-                    cpu = fmt::format(" [ CPU {} ]", cpus.begin()->as_int());
+                    cpu = fmt::format(" [ CPU {} ]", *cpus_int.begin());
                 }
                 else
                 {
-                    cpu = fmt::format(" [ CPUs {}-{} ]",
-                                      std::min_element(cpus.begin(), cpus.end())->as_int(),
-                                      std::max_element(cpus.begin(), cpus.end())->as_int());
+                    int min_cpu = *std::min_element(cpus_int.begin(), cpus_int.end());
+                    int max_cpu = *std::max_element(cpus_int.begin(), cpus_int.end());
+
+                    if (max_cpu - min_cpu + 1 == static_cast<int>(cpus_int.size()))
+                    {
+                        cpu = fmt::format(" [ CPUs {}-{} ]", min_cpu, max_cpu);
+                    }
+                    else
+                    {
+                        cpu = fmt::format(" [ CPUs {}]", fmt::join(cpus_int, ", "));
+                    }
                 }
             }
         }
@@ -696,8 +708,9 @@ void parse_program_options(int argc, const char** argv)
     }
 
     // time synchronization
-    config.use_clockid = false;
     config.use_pebs = false;
+    config.clockid = std::nullopt;
+
     try
     {
         std::string requested_clock_name = arguments.get("clockid");
@@ -713,7 +726,6 @@ void parse_program_options(int argc, const char** argv)
 
         lo2s::Log::debug() << "Using clock \'" << clock.name << "\'.";
 #ifndef USE_HW_BREAKPOINT_COMPAT
-        config.use_clockid = true;
         config.clockid = clock.id;
 #else
         if (requested_clock_name != "monotonic-raw")

src/monitor/process_monitor_main.cpp

@@ -158,9 +158,9 @@ std::vector<char*> to_vector_of_c_str(const std::vector<std::string>& vec)
     {
         env = { "CUDA_INJECTION64_PATH=" + config().cuda_injectionlib_path };
 
-        if (config().use_clockid)
+        if (config().clockid)
         {
-            env.push_back("LO2S_CLOCKID=" + std::to_string(config().clockid));
+            env.push_back("LO2S_CLOCKID=" + std::to_string(config().clockid.value()));
         }
     }
 #endif

src/perf/event_attr.cpp

@@ -130,7 +130,7 @@ static constexpr std::uint64_t apply_mask(std::uint64_t value, std::uint64_t mas
     return res;
 }
 
-EventAttr::EventAttr(const std::string name, perf_type_id type, std::uint64_t config,
+EventAttr::EventAttr(const std::string& name, perf_type_id type, std::uint64_t config,
                      std::uint64_t config1)
 
 : name_(name)
@@ -143,7 +143,7 @@ EventAttr::EventAttr(const std::string name, perf_type_id type, std::uint64_t co
     attr_.config1 = config1;
 }
 
-SimpleEventAttr::SimpleEventAttr(const std::string name, perf_type_id type, std::uint64_t config,
+SimpleEventAttr::SimpleEventAttr(const std::string& name, perf_type_id type, std::uint64_t config,
 
                                  std::uint64_t config1)
 : EventAttr(name, type, config, config1)
@@ -282,7 +282,7 @@ void EventAttr::update_availability()
     }
 }
 
-SimpleEventAttr SimpleEventAttr::raw(std::string ev_name)
+SimpleEventAttr SimpleEventAttr::raw(const std::string& ev_name)
 {
     // Do not check whether the event_is_openable because we don't know whether we are in
     // system or process mode

src/perf/event_composer.cpp

@@ -71,7 +71,7 @@ EventAttr EventComposer::create_sampling_event()
 
         if (config().use_pebs)
         {
-            sampling_event_->set_clockid(-1);
+            sampling_event_->set_clockid(std::nullopt);
         }
         else
         {
@@ -158,14 +158,15 @@ std::vector<perf::tracepoint::TracepointEventAttr> EventComposer::get_tracepoint
     }
 
     tracepoint_events_ = std::vector<tracepoint::TracepointEventAttr>();
-    for (auto& ev_name : config().tracepoint_events)
+    for (const auto& ev_name : config().tracepoint_events)
     {
         tracepoint_events_->emplace_back(create_tracepoint_event(ev_name));
     }
     return tracepoint_events_.value();
 }
 
-perf::tracepoint::TracepointEventAttr EventComposer::create_tracepoint_event(std::string name)
+perf::tracepoint::TracepointEventAttr
+EventComposer::create_tracepoint_event(const std::string& name)
 {
     auto ev = tracepoint::TracepointEventAttr(name);
     watermark(ev);

src/perf/event_resolver.cpp

@@ -337,7 +337,7 @@ EventAttr EventResolver::get_event_by_name(const std::string& name)
     }
 }
 
-EventAttr EventResolver::get_metric_leader(std::string metric_leader)
+EventAttr EventResolver::get_metric_leader(const std::string& metric_leader)
 {
     std::optional<EventAttr> leader;
     Log::info() << "choosing default metric-leader";

src/perf/time/converter.cpp

@@ -44,7 +44,7 @@ Converter::Converter() : offset(otf2::chrono::duration(0))
     const auto time_diff =
         reader.local_time.time_since_epoch() - reader.perf_time.time_since_epoch();
 
-    if (lo2s::config().use_clockid)
+    if (lo2s::config().clockid.has_value())
     {
         if (time_diff < std::chrono::microseconds(-100) or time_diff > std::chrono::microseconds(0))
         {