-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(EventDescription): Turn EventDescription into real class #276
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -21,7 +21,10 @@ | |||||
|
||||||
#pragma once | ||||||
|
||||||
#include <lo2s/config.hpp> | ||||||
#include <lo2s/error.hpp> | ||||||
#include <lo2s/execution_scope.hpp> | ||||||
#include <lo2s/perf/util.hpp> | ||||||
#include <lo2s/topology.hpp> | ||||||
|
||||||
#include <set> | ||||||
|
@@ -48,16 +51,54 @@ struct EventDescription | |||||
{ | ||||||
EventDescription(const std::string& name, perf_type_id type, std::uint64_t config, | ||||||
std::uint64_t config1 = 0, std::set<Cpu> cpus = std::set<Cpu>(), | ||||||
double scale = 1, std::string unit = "#", | ||||||
Availability availability = Availability::UNAVAILABLE) | ||||||
: name(name), type(type), config(config), config1(config1), scale(scale), unit(unit), | ||||||
availability(availability), cpus_(cpus) | ||||||
double scale = 1, std::string unit = "#") | ||||||
: name_(name), type_(type), config_(config), config1_(config1), scale_(scale), unit_(unit), | ||||||
cpus_(cpus) | ||||||
{ | ||||||
struct perf_event_attr attr = perf_event_attr(); | ||||||
|
||||||
int proc_fd = perf_event_open(&attr, ExecutionScope(Thread(0)), -1, 0); | ||||||
int sys_fd = perf_event_open(&attr, ExecutionScope(*supported_cpus().begin()), -1, 0); | ||||||
|
||||||
if (sys_fd == -1 && proc_fd == -1) | ||||||
{ | ||||||
attr.exclude_kernel = 1; | ||||||
proc_fd = perf_event_open(&attr, ExecutionScope(Thread(0)), -1, 0); | ||||||
sys_fd = perf_event_open(&attr, ExecutionScope(*supported_cpus().begin()), -1, 0); | ||||||
} | ||||||
|
||||||
if (sys_fd == -1 && proc_fd == -1) | ||||||
{ | ||||||
switch (errno) | ||||||
{ | ||||||
case ENOTSUP: | ||||||
Log::debug() << "perf event not supported by the running kernel: " << name_; | ||||||
break; | ||||||
default: | ||||||
Log::debug() << "perf event " << name_ | ||||||
<< " not available: " << std::string(std::strerror(errno)); | ||||||
break; | ||||||
} | ||||||
|
||||||
availability_ = Availability::UNAVAILABLE; | ||||||
} | ||||||
else if (sys_fd == -1) | ||||||
{ | ||||||
availability_ = Availability::PROCESS_MODE; | ||||||
} | ||||||
else if (proc_fd == -1) | ||||||
{ | ||||||
availability_ = Availability::SYSTEM_MODE; | ||||||
} | ||||||
else | ||||||
{ | ||||||
availability_ = Availability::UNIVERSAL; | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need an |
||||||
} | ||||||
|
||||||
EventDescription() | ||||||
: name(""), type(static_cast<perf_type_id>(-1)), config(0), config1(0), scale(1), unit("#"), | ||||||
availability(Availability::UNAVAILABLE) | ||||||
: name_(""), type_(static_cast<perf_type_id>(-1)), config_(0), config1_(0), scale_(1), | ||||||
unit_("#"), availability_(Availability::UNAVAILABLE) | ||||||
{ | ||||||
} | ||||||
|
||||||
|
@@ -79,30 +120,106 @@ struct EventDescription | |||||
|
||||||
friend bool operator==(const EventDescription& lhs, const EventDescription& rhs) | ||||||
{ | ||||||
return (lhs.type == rhs.type) && (lhs.config == rhs.config) && (lhs.config1 == rhs.config1); | ||||||
return (lhs.type_ == rhs.type_) && (lhs.config_ == rhs.config_) && | ||||||
(lhs.config1_ == rhs.config1_); | ||||||
} | ||||||
|
||||||
friend bool operator<(const EventDescription& lhs, const EventDescription& rhs) | ||||||
{ | ||||||
if (lhs.type == rhs.type) | ||||||
if (lhs.type_ == rhs.type_) | ||||||
{ | ||||||
if (lhs.config == rhs.config) | ||||||
if (lhs.config_ == rhs.config_) | ||||||
{ | ||||||
return lhs.config1 < rhs.config1; | ||||||
return lhs.config1_ < rhs.config1_; | ||||||
} | ||||||
return lhs.config < rhs.config; | ||||||
return lhs.config_ < rhs.config_; | ||||||
} | ||||||
return lhs.type_ < rhs.type_; | ||||||
} | ||||||
|
||||||
struct perf_event_attr perf_event_attr() const | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Should just be called |
||||||
{ | ||||||
struct perf_event_attr attr; | ||||||
memset(&attr, 0, sizeof(struct perf_event_attr)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, missing header: |
||||||
|
||||||
attr.size = sizeof(struct perf_event_attr); | ||||||
|
||||||
attr.type = type_; | ||||||
attr.config = config_; | ||||||
attr.config1 = config1_; | ||||||
|
||||||
return attr; | ||||||
} | ||||||
|
||||||
std::string name() const | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
return name_; | ||||||
} | ||||||
|
||||||
std::string description() const | ||||||
{ | ||||||
if (availability_ == Availability::UNIVERSAL) | ||||||
{ | ||||||
return name_; | ||||||
} | ||||||
else if (availability_ == Availability::SYSTEM_MODE) | ||||||
{ | ||||||
return fmt::format("{} [SYS]", name_); | ||||||
} | ||||||
else if (availability_ == Availability::PROCESS_MODE) | ||||||
{ | ||||||
return fmt::format("{} [PROC]", name_); | ||||||
} | ||||||
|
||||||
return ""; | ||||||
} | ||||||
|
||||||
bool is_valid() const | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
return availability_ != Availability::UNAVAILABLE; | ||||||
} | ||||||
|
||||||
double scale() const | ||||||
{ | ||||||
return scale_; | ||||||
} | ||||||
|
||||||
std::string unit() const | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
return unit_; | ||||||
} | ||||||
|
||||||
int open_counter(ExecutionScope scope, int group_fd) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really need an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having said that. I don't like the fact, that some parts of the code use this method and others are using |
||||||
{ | ||||||
struct perf_event_attr perf_attr = perf_event_attr(); | ||||||
perf_attr.sample_period = 0; | ||||||
perf_attr.exclude_kernel = config().exclude_kernel; | ||||||
// Needed when scaling multiplexed events, and recognize activation phases | ||||||
perf_attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_TOTAL_TIME_RUNNING; | ||||||
|
||||||
#ifndef USE_HW_BREAKPOINT_COMPAT | ||||||
perf_attr.use_clockid = config().use_clockid; | ||||||
perf_attr.clockid = config().clockid; | ||||||
#endif | ||||||
|
||||||
int fd = perf_try_event_open(&perf_attr, scope, group_fd, 0, config().cgroup_fd); | ||||||
if (fd < 0) | ||||||
{ | ||||||
Log::error() << "perf_event_open for counter failed"; | ||||||
throw_errno(); | ||||||
} | ||||||
return lhs.type < rhs.type; | ||||||
return fd; | ||||||
} | ||||||
std::string name; | ||||||
perf_type_id type; | ||||||
std::uint64_t config; | ||||||
std::uint64_t config1; | ||||||
double scale; | ||||||
std::string unit; | ||||||
Availability availability; | ||||||
|
||||||
private: | ||||||
std::string name_; | ||||||
perf_type_id type_; | ||||||
std::uint64_t config_; | ||||||
std::uint64_t config1_; | ||||||
double scale_; | ||||||
std::string unit_; | ||||||
Availability availability_; | ||||||
|
||||||
std::set<Cpu> cpus_; | ||||||
}; | ||||||
} // namespace perf | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,15 +84,7 @@ class Reader : public EventReader<T> | |
Log::debug() << "initializing event_reader for:" << scope.name() | ||
<< ", enable_on_exec: " << enable_on_exec; | ||
|
||
struct perf_event_attr perf_attr = common_perf_event_attrs(); | ||
|
||
if (config().use_pebs) | ||
{ | ||
perf_attr.use_clockid = 0; | ||
} | ||
|
||
perf_attr.exclude_kernel = config().exclude_kernel; | ||
perf_attr.sample_period = config().sampling_period; | ||
struct perf_event_attr perf_attr; | ||
|
||
if (config().sampling) | ||
{ | ||
|
@@ -103,19 +95,31 @@ class Reader : public EventReader<T> | |
|
||
Log::debug() << "using sampling event \'" << config().sampling_event | ||
<< "\', period: " << config().sampling_period; | ||
|
||
perf_attr.type = sampling_event.type; | ||
perf_attr.config = sampling_event.config; | ||
perf_attr.config1 = sampling_event.config1; | ||
perf_attr = sampling_event.perf_event_attr(); | ||
|
||
perf_attr.mmap = 1; | ||
perf_attr.disabled = 1; | ||
|
||
#ifndef USE_HW_BREAKPOINT_COMPAT | ||
perf_attr.use_clockid = config().use_clockid; | ||
perf_attr.clockid = config().clockid; | ||
#endif | ||
// 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 | ||
perf_attr.watermark = 1; | ||
perf_attr.wakeup_watermark = | ||
static_cast<uint32_t>(0.8 * config().mmap_pages * get_page_size()); | ||
Comment on lines
+103
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🍝 |
||
} | ||
else | ||
{ | ||
perf_attr = common_perf_event_attrs(); | ||
|
||
// Set up a dummy event for recording calling context enter/leaves only | ||
perf_attr.type = PERF_TYPE_SOFTWARE; | ||
perf_attr.config = PERF_COUNT_SW_DUMMY; | ||
} | ||
perf_attr.exclude_kernel = config().exclude_kernel; | ||
perf_attr.sample_period = config().sampling_period; | ||
|
||
perf_attr.sample_id_all = 1; | ||
// Generate PERF_RECORD_COMM events to trace changes to the command | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we just open it with
exclude_kernel
, if it's for testing only anyways?