From 60fe8b7bcf915468ec39d5f17cbd50b168c490bd Mon Sep 17 00:00:00 2001 From: Tessa Todorowski Date: Fri, 23 Aug 2024 08:38:03 +0200 Subject: [PATCH] refactor event getter & setter --- .../lo2s/perf/counter/counter_collection.hpp | 4 +- include/lo2s/perf/event.hpp | 31 ++++++++------ include/lo2s/perf/sample/reader.hpp | 8 ++-- include/lo2s/trace/trace.hpp | 5 +-- src/config.cpp | 8 ++-- src/monitor/tracepoint_monitor.cpp | 4 +- src/perf/counter/counter_provider.cpp | 8 ++-- src/perf/counter/group/reader.cpp | 16 ++++--- src/perf/counter/userspace/reader.cpp | 4 +- src/perf/event.cpp | 10 ++--- src/perf/event_provider.cpp | 42 +++++++++---------- src/trace/trace.cpp | 8 ++-- 12 files changed, 75 insertions(+), 73 deletions(-) diff --git a/include/lo2s/perf/counter/counter_collection.hpp b/include/lo2s/perf/counter/counter_collection.hpp index f1690a24..d6bf6f0f 100644 --- a/include/lo2s/perf/counter/counter_collection.hpp +++ b/include/lo2s/perf/counter/counter_collection.hpp @@ -40,11 +40,11 @@ struct CounterCollection { if (index == 0) { - return leader.get_scale(); + return leader.scale(); } else { - return counters[index - 1].get_scale(); + return counters[index - 1].scale(); } } diff --git a/include/lo2s/perf/event.hpp b/include/lo2s/perf/event.hpp index 4effac46..5acc09f4 100644 --- a/include/lo2s/perf/event.hpp +++ b/include/lo2s/perf/event.hpp @@ -82,47 +82,52 @@ class Event */ EventGuard open_as_group_leader(ExecutionScope location, int cgroup_fd = -1); - const Availability& get_availability() const + const Availability& availability() const { return availability_; }; - std::string get_name() const + std::string name() const { return name_; } - std::set get_cpus() const + std::set cpus() const { return cpus_; } - std::string get_unit() const + std::string unit() const { return unit_; } - perf_event_attr& get_attr() + const perf_event_attr& attr() const { return attr_; } - double get_scale() const + perf_event_attr& mut_attr() + { + return attr_; + } + + double scale() const { return scale_; } - void set_scale(double scale) + void scale(double scale) { scale_ = scale; } - void set_unit(const std::string& unit) + void unit(const std::string& unit) { unit_ = unit; } - void set_clock_attrs([[maybe_unused]] bool use_clockid, [[maybe_unused]] clockid_t clockid) + void clock_attrs([[maybe_unused]] bool use_clockid, [[maybe_unused]] clockid_t clockid) { #ifndef USE_HW_BREAKPOINT_COMPAT attr_.use_clockid = use_clockid; @@ -132,19 +137,19 @@ class Event // When we poll on the fd given by perf_event_open, wakeup, when our buffer is 80% full // Default behaviour is to wakeup on every event, which is horrible performance wise - void set_watermark(size_t mmap_pages) + void watermark(size_t mmap_pages) { attr_.watermark = 1; attr_.wakeup_watermark = static_cast(0.8 * mmap_pages * sysconf(_SC_PAGESIZE)); } - void set_exclude_kernel(bool exclude_kernel) + void exclude_kernel(bool exclude_kernel) { attr_.exclude_kernel = exclude_kernel; } - void set_sample_period(const int& period); - void set_sample_freq(const uint64_t& freq); + void sample_period(const int& period); + void sample_freq(const uint64_t& freq); void event_attr_update(std::uint64_t value, const std::string& format); void parse_pmu_path(const std::string& ev_name); diff --git a/include/lo2s/perf/sample/reader.hpp b/include/lo2s/perf/sample/reader.hpp index 1ba9bc14..0f5a780f 100644 --- a/include/lo2s/perf/sample/reader.hpp +++ b/include/lo2s/perf/sample/reader.hpp @@ -94,10 +94,10 @@ class Reader : public EventReader } catch (const std::system_error& e) { - if (e.code().value() == EACCES && !event.get_attr().exclude_kernel && + if (e.code().value() == EACCES && !event.attr().exclude_kernel && perf_event_paranoid() > 1) { - event.get_attr().exclude_kernel = 1; + event.mut_attr().exclude_kernel = 1; perf_warn_paranoid(); continue; } @@ -106,7 +106,7 @@ class Reader : public EventReader { Log::error() << "perf_event_open for sampling failed: " << e.what(); - if (event.get_attr().use_clockid) + if (event.attr().use_clockid) { Log::error() << "maybe the specified clock is unavailable?"; } @@ -115,7 +115,7 @@ class Reader : public EventReader } } while (!event_.is_valid()); - Log::debug() << "Using precise_ip level: " << event.get_attr().precise_ip; + Log::debug() << "Using precise_ip level: " << event.attr().precise_ip; // Exception safe, so much wow! try diff --git a/include/lo2s/trace/trace.hpp b/include/lo2s/trace/trace.hpp index 698c497d..26bcf06b 100644 --- a/include/lo2s/trace/trace.hpp +++ b/include/lo2s/trace/trace.hpp @@ -174,10 +174,9 @@ class Trace otf2::definition::metric_member& get_event_metric_member(perf::Event event) { return registry_.emplace( - BySamplingEvent(event), intern(event.get_name()), intern(event.get_name()), + BySamplingEvent(event), intern(event.name()), intern(event.name()), otf2::common::metric_type::other, otf2::common::metric_mode::accumulated_start, - otf2::common::type::Double, otf2::common::base_type::decimal, 0, - intern(event.get_unit())); + otf2::common::type::Double, otf2::common::base_type::decimal, 0, intern(event.unit())); } otf2::definition::metric_class& perf_metric_class(MeasurementScope scope) diff --git a/src/config.cpp b/src/config.cpp index 017b931e..936da400 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -84,11 +84,11 @@ static inline void print_availability(std::ostream& os, const std::string& descr std::string availability = ""; std::string cpu = ""; - if (ev.get_availability() == perf::Availability::PROCESS_MODE) + if (ev.availability() == perf::Availability::PROCESS_MODE) { availability = " *"; } - else if (ev.get_availability() == perf::Availability::SYSTEM_MODE) + else if (ev.availability() == perf::Availability::SYSTEM_MODE) { availability = " #"; } @@ -100,7 +100,7 @@ static inline void print_availability(std::ostream& os, const std::string& descr std::max_element(cpus.begin(), cpus.end())->as_int()); } - event_names.push_back(ev.get_name() + availability + cpu); + event_names.push_back(ev.name() + availability + cpu); } list_arguments_sorted(os, description, event_names); } @@ -731,7 +731,7 @@ void parse_program_options(int argc, const char** argv) { for (const auto& mem_event : platform::get_mem_events()) { - perf_group_events.emplace_back(mem_event.get_name()); + perf_group_events.emplace_back(mem_event.name()); } perf_group_events.emplace_back("instructions"); perf_group_events.emplace_back("cpu-cycles"); diff --git a/src/monitor/tracepoint_monitor.cpp b/src/monitor/tracepoint_monitor.cpp index 0d937598..35bbc37d 100644 --- a/src/monitor/tracepoint_monitor.cpp +++ b/src/monitor/tracepoint_monitor.cpp @@ -42,9 +42,9 @@ TracepointMonitor::TracepointMonitor(trace::Trace& trace, Cpu cpu) for (const auto& event : tracepoint_collection.counters) { - auto& mc = trace.tracepoint_metric_class(event.get_name()); + auto& mc = trace.tracepoint_metric_class(event.name()); std::unique_ptr writer = - std::make_unique(cpu, event.get_name(), trace, mc); + std::make_unique(cpu, event.name(), trace, mc); add_fd(writer->fd()); perf_writers_.emplace(std::piecewise_construct, std::forward_as_tuple(writer->fd()), diff --git a/src/perf/counter/counter_provider.cpp b/src/perf/counter/counter_provider.cpp index 91bf8d75..f46815ab 100644 --- a/src/perf/counter/counter_provider.cpp +++ b/src/perf/counter/counter_provider.cpp @@ -61,7 +61,7 @@ void CounterProvider::initialize_userspace_counters(const std::vector::Reader(ExecutionScope scope, bool enable_on_exec) { if (config().metric_use_frequency) { - counter_collection_.leader.set_sample_freq(config().metric_frequency); + counter_collection_.leader.sample_freq(config().metric_frequency); } else { - counter_collection_.leader.set_sample_period(config().metric_count); + counter_collection_.leader.sample_period(config().metric_count); } do @@ -73,9 +73,9 @@ Reader::Reader(ExecutionScope scope, bool enable_on_exec) { // perf_try_event_open was used here before if (counter_leader_.get_fd() < 0 && errno == EACCES && - !counter_collection_.leader.get_attr().exclude_kernel && perf_event_paranoid() > 1) + !counter_collection_.leader.attr().exclude_kernel && perf_event_paranoid() > 1) { - counter_collection_.leader.get_attr().exclude_kernel = 1; + counter_collection_.leader.mut_attr().exclude_kernel = 1; perf_warn_paranoid(); continue; @@ -89,16 +89,14 @@ Reader::Reader(ExecutionScope scope, bool enable_on_exec) } } while (!counter_leader_.is_valid()); - Log::debug() << "counter::Reader: leader event: '" << counter_collection_.leader.get_name() - << "'"; + Log::debug() << "counter::Reader: leader event: '" << counter_collection_.leader.name() << "'"; for (auto& counter_ev : counter_collection_.counters) { if (counter_ev.is_available_in(scope)) { EventGuard counter; - counter_ev.get_attr().exclude_kernel = - counter_collection_.leader.get_attr().exclude_kernel; + counter_ev.mut_attr().exclude_kernel = counter_collection_.leader.attr().exclude_kernel; do { try @@ -109,7 +107,7 @@ Reader::Reader(ExecutionScope scope, bool enable_on_exec) { if (!counter.is_valid()) { - Log::error() << "failed to add counter '" << counter_ev.get_name() + Log::error() << "failed to add counter '" << counter_ev.name() << "': " << e.code().message(); if (e.code().value() == EINVAL) diff --git a/src/perf/counter/userspace/reader.cpp b/src/perf/counter/userspace/reader.cpp index a4ba2174..94431052 100644 --- a/src/perf/counter/userspace/reader.cpp +++ b/src/perf/counter/userspace/reader.cpp @@ -75,10 +75,10 @@ Reader::Reader(ExecutionScope scope) catch (const std::system_error& e) { // perf_try_event_open was used here before - if (counter.get_fd() < 0 && errno == EACCES && !event.get_attr().exclude_kernel && + if (counter.get_fd() < 0 && errno == EACCES && !event.attr().exclude_kernel && perf_event_paranoid() > 1) { - event.get_attr().exclude_kernel = 1; + event.mut_attr().exclude_kernel = 1; perf_warn_paranoid(); counter = event.open(scope); diff --git a/src/perf/event.cpp b/src/perf/event.cpp index 42d85d82..aa669ccd 100644 --- a/src/perf/event.cpp +++ b/src/perf/event.cpp @@ -283,13 +283,13 @@ void Event::parse_cpus() [](uint32_t cpuid) { return Cpu(cpuid); }); } -void Event::set_sample_period(const int& period) +void Event::sample_period(const int& period) { Log::debug() << "counter::Reader: sample_period: " << period; attr_.sample_period = period; } -void Event::set_sample_freq(const uint64_t& freq) +void Event::sample_freq(const uint64_t& freq) { Log::debug() << "counter::Reader: sample_freq: " << freq; attr_.sample_freq = freq; @@ -495,8 +495,8 @@ SysfsEvent::SysfsEvent(const std::string& ev_name, bool enable_on_exec) : Event( << name_ << "/type=" << attr_.type << ",config=" << attr_.config << ",config1=" << attr_.config1 << std::dec << std::noshowbase << "/"; - set_scale(read_file_or_else(event_path.replace_extension(".scale"), 1.0)); - set_unit(read_file_or_else(event_path.replace_extension(".unit"), "#")); + scale(read_file_or_else(event_path.replace_extension(".scale"), 1.0)); + unit(read_file_or_else(event_path.replace_extension(".unit"), "#")); if (!event_is_openable()) { @@ -596,7 +596,7 @@ EventGuard::EventGuard(Event& ev, std::variant location, int group_ [&](Thread thread) { scope = thread.as_scope(); } }, location); - fd_ = perf_event_open(&ev_.get_attr(), scope, group_fd, 0, cgroup_fd); + fd_ = perf_event_open(&ev_.mut_attr(), scope, group_fd, 0, cgroup_fd); if (fd_ < 0) { diff --git a/src/perf/event_provider.cpp b/src/perf/event_provider.cpp index 075386f2..83b467bf 100644 --- a/src/perf/event_provider.cpp +++ b/src/perf/event_provider.cpp @@ -154,13 +154,13 @@ static void populate_event_map(std::unordered_map& map) for (auto& ev : HW_EVENT_TABLE) { Event event(ev); - map.emplace(event.get_name(), event); + map.emplace(event.name(), event); } for (auto& ev : SW_EVENT_TABLE) { Event event(ev); - map.emplace(event.get_name(), event); + map.emplace(event.name(), event); } std::stringstream name_fmt; @@ -175,10 +175,10 @@ static void populate_event_map(std::unordered_map& map) Event event(name_fmt.str(), PERF_TYPE_HW_CACHE, make_cache_config(cache.id, operation.id.op_id, operation.id.result_id)); - event.set_sample_period(0); - event.set_exclude_kernel(true); - event.set_watermark(16); - event.set_clock_attrs(true, CLOCK_MONOTONIC_RAW); + event.sample_period(0); + event.exclude_kernel(true); + event.watermark(16); + event.clock_attrs(true, CLOCK_MONOTONIC_RAW); map.emplace(name_fmt.str(), event); } @@ -372,7 +372,7 @@ tracepoint::TracepointEvent EventProvider::create_tracepoint_event(const std::st bool enable_on_exec) { tracepoint::TracepointEvent event(name, enable_on_exec); - event.set_sample_period(0); + event.sample_period(0); apply_config_attrs(event); return event; @@ -382,7 +382,7 @@ tracepoint::TracepointEvent EventProvider::create_raw_tracepoint_event(const std bool enable_on_exec) { tracepoint::TracepointEvent event(name, enable_on_exec); - event.set_sample_period(0); + event.sample_period(0); apply_default_attrs(event); return event; @@ -391,10 +391,10 @@ tracepoint::TracepointEvent EventProvider::create_raw_tracepoint_event(const std Event EventProvider::create_time_event(uint64_t local_time) { Event event(local_time); - event.set_sample_period(1); + event.sample_period(1); apply_config_attrs(event); - event.set_exclude_kernel(true); // overwrite config value + event.exclude_kernel(true); // overwrite config value return event; } @@ -403,10 +403,10 @@ Event EventProvider::create_raw_event(const std::string& name, perf_type_id type std::uint64_t config, std::uint64_t config1) { Event event(name, type, config, config1); - event.set_sample_period(0); + event.sample_period(0); apply_config_attrs(event); - event.set_exclude_kernel(true); // overwrite config value + event.exclude_kernel(true); // overwrite config value return event; } @@ -416,7 +416,7 @@ SysfsEvent EventProvider::create_sampling_event(bool enable_on_exec) SysfsEvent event(config().sampling_event, enable_on_exec); apply_config_attrs(event); - event.set_sample_period(config().sampling_period); + event.sample_period(config().sampling_period); event.use_sampling_options(config().use_pebs, config().sampling, config().enable_cct); return event; @@ -425,7 +425,7 @@ SysfsEvent EventProvider::create_sampling_event(bool enable_on_exec) SysfsEvent EventProvider::create_raw_sysfs_event(const std::string& name) { SysfsEvent event(name); - event.set_sample_period(0); + event.sample_period(0); apply_default_attrs(event); return event; @@ -434,7 +434,7 @@ SysfsEvent EventProvider::create_raw_sysfs_event(const std::string& name) SysfsEvent EventProvider::create_sysfs_event(const std::string& name) { SysfsEvent event(name); - event.set_sample_period(0); + event.sample_period(0); apply_config_attrs(event); return event; @@ -442,16 +442,16 @@ SysfsEvent EventProvider::create_sysfs_event(const std::string& name) void EventProvider::apply_config_attrs(Event& event) { - event.set_watermark(config().mmap_pages); - event.set_exclude_kernel(config().exclude_kernel); - event.set_clock_attrs(config().use_clockid, config().clockid); + event.watermark(config().mmap_pages); + event.exclude_kernel(config().exclude_kernel); + event.clock_attrs(config().use_clockid, config().clockid); } void EventProvider::apply_default_attrs(Event& event) { - event.set_watermark(16); // default mmap-pages value - event.set_exclude_kernel(true); // enabled by default - event.set_clock_attrs(true, CLOCK_MONOTONIC_RAW); + event.watermark(16); // default mmap-pages value + event.exclude_kernel(true); // enabled by default + event.clock_attrs(true, CLOCK_MONOTONIC_RAW); } } // namespace perf diff --git a/src/trace/trace.cpp b/src/trace/trace.cpp index afe9f84f..f673b154 100644 --- a/src/trace/trace.cpp +++ b/src/trace/trace.cpp @@ -559,17 +559,17 @@ Trace::metric_instance(const otf2::definition::metric_class& metric_class, otf2::definition::metric_class& Trace::tracepoint_metric_class(const perf::tracepoint::TracepointEvent& event) { - if (!registry_.has(ByString(event.get_name()))) + if (!registry_.has(ByString(event.name()))) { auto& mc = registry_.create( - ByString(event.get_name()), otf2::common::metric_occurence::async, + ByString(event.name()), otf2::common::metric_occurence::async, otf2::common::recorder_kind::abstract); for (const auto& field : event.fields()) { if (field.is_integer()) { - mc.add_member(metric_member(event.get_name() + "::" + field.name(), "?", + mc.add_member(metric_member(event.name() + "::" + field.name(), "?", otf2::common::metric_mode::absolute_next, otf2::common::type::int64, "#")); } @@ -578,7 +578,7 @@ Trace::tracepoint_metric_class(const perf::tracepoint::TracepointEvent& event) } else { - return registry_.get(ByString(event.get_name())); + return registry_.get(ByString(event.name())); } }