fix(EventDescription): Turn EventDescription into real class #276
Conversation
cvonelm
force-pushed
the
issue-224-decify-event-description
branch
from
838638e
to
dc2d7bf
Compare
|
I'm out of the loop about this. Can you extend the PR description (and commit message) and elaborate on what this PR tries to achieve? |
|
I will be updating the commit message once this is ready for merging and squashed. |
| else | ||
| { | ||
| availability_ = Availability::UNIVERSAL; | ||
| } |
There was a problem hiding this comment.
We need an Fd RAII object. sys_fd and proc_fd are never closed, if they succeed in opening.
|
|
||
| if (sys_fd == -1 && proc_fd == -1) | ||
| { | ||
| attr.exclude_kernel = 1; |
There was a problem hiding this comment.
Why don't we just open it with exclude_kernel, if it's for testing only anyways?
| struct perf_event_attr perf_event_attr() const | ||
| { | ||
| struct perf_event_attr attr; | ||
| memset(&attr, 0, sizeof(struct perf_event_attr)); |
There was a problem hiding this comment.
| memset(&attr, 0, sizeof(struct perf_event_attr)); | |
| std::memset(&attr, 0, sizeof(struct perf_event_attr)); |
| return attr; | ||
| } | ||
|
|
||
| std::string name() const |
There was a problem hiding this comment.
| std::string name() const | |
| const& std::string name() const |
| return scale_; | ||
| } | ||
|
|
||
| std::string unit() const |
There was a problem hiding this comment.
| std::string unit() const | |
| const& std::string unit() const |
| static constexpr auto npos = std::string::npos; | ||
| const auto colon = format.find_first_of(':'); | ||
| if (colon == npos) | ||
| { | ||
| throw EventProvider::InvalidEvent("invalid format description: missing colon"); | ||
| } | ||
|
|
||
| const auto target_config = format.substr(0, colon); | ||
| const auto mask = parse_bitmask(format.substr(colon + 1)); | ||
|
|
||
| if (target_config == "config") | ||
| { | ||
| config |= apply_mask(val, mask); | ||
| } | ||
|
|
||
| if (target_config == "config1") | ||
| { | ||
| config1 |= apply_mask(val, mask); | ||
| } |
| std::underlying_type<perf_type_id>::type type; | ||
| uint64_t config = 0; | ||
| uint64_t config1 = 0; | ||
| std::set<Cpu> cpus; | ||
| double scale; | ||
| std::string unit; | ||
|
|
There was a problem hiding this comment.
Why is this here?
Seems to be unused for another 100 lines of code.
| constexpr std::uint64_t bit(int bitnumber) | ||
| { | ||
| return static_cast<std::uint64_t>(1_u64 << bitnumber); | ||
| return static_cast<std::uint64_t>(1ULL << bitnumber); |
There was a problem hiding this comment.
| return static_cast<std::uint64_t>(1ULL << bitnumber); | |
| return 1ull << bitnumber; |
| static constexpr auto npos = std::string::npos; | ||
| const auto colon = format.find_first_of(':'); | ||
| if (colon == npos) |
There was a problem hiding this comment.
| static constexpr auto npos = std::string::npos; | |
| const auto colon = format.find_first_of(':'); | |
| if (colon == npos) | |
| const auto colon = format.find_first_of(':'); | |
| if (colon == std::string::npos) |
There was a problem hiding this comment.
This should be have been a regex in the first place. What about format == "foo:"?
| leader_attr.disabled = 1; | ||
|
|
||
| #ifndef USE_HW_BREAKPOINT_COMPAT | ||
| leader_attr.use_clockid = config().use_clockid; | ||
| leader_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 | ||
| leader_attr.watermark = 1; | ||
| leader_attr.wakeup_watermark = | ||
| static_cast<uint32_t>(0.8 * config().mmap_pages * get_page_size()); |
|
Dropping this PR in favour of a more thorough refactoring based on the raii-fd branch. |
The current EventDescription is a mere struct with exposed struct members etc. This PR attempts to rework the EventDescription into a proper class while integrating some of the Event Handling Code into this new class.
This fixes #224
This also fixes #275, a bug that prevents events from showing if they could only be opened with exclude_kernel=1