traced_perf - support setting period/frequency for follower events.#5526
traced_perf - support setting period/frequency for follower events.#5526abhmen01 wants to merge 1 commit intogoogle:mainfrom
Conversation
rsavitski
left a comment
There was a problem hiding this comment.
I feel like this patch is motivated due to a misunderstanding of the current config, and lacks a big chunk of implementation of the buffer sharing part.
Currently, each "perf.linux" data source is one recording group that uses leader sampling (https://man7.org/linux/man-pages/man1/perf-list.1.html#LEADER_SAMPLING). So only the leader sampling is by design.
If you need to independently record several counters, you can specify several "linux.perf" data sources in your config. That's the equivalent of "perf record -e A,B,C".
|
|
||
| // redirect the follower events to the leader's buffer. | ||
| for (size_t i = 0; i < follower_fds.size(); ++i) { | ||
| ioctl(follower_fds[i].get(), PERF_EVENT_IOC_SET_OUTPUT, timebase_fd.get()); |
There was a problem hiding this comment.
This seems very incomplete. It redirects events, but the patch doesn't have any code for adjusting how events are read when using either the sampling or the polling interface.
I'd expect a lot more code for any buffer sharing. In particular extra IDs in the samples (see PERF_SAMPLE_STREAM_ID) and demultiplexing them on the reading side.
What kind of config were you testing with?
There was a problem hiding this comment.
For the grouped case, I did not think extra demultiplexing was needed, because we always use PERF_FORMAT_GROUP, so the whole group is read together regardless of which event triggered the overflow. In that path, PERF_SAMPLE_STREAM_ID should not be required for decoding, which is why I did not add it.
On further review, I think the non-grouped case still needs to be handled explicitly, even though PERF_FORMAT_GROUP is always enabled during configuration. There we would need to maintain a mapping from event index to stream ID and use that during decoding.
There was a problem hiding this comment.
Modified the events reading code to use the stream id to recognize the event which produced the sample.
|
The current behavior is indeed leader sampling, and I agree that this is how perfetto works today. So today the behavior is effectively equivalent to: With this patch, if a period is specified for follower That is, the group is still read as a unit regardless of which event triggered the sample, but sampling is no longer restricted to the leader only. So I think the key difference is: current behavior: grouped read, leader-only sample trigger |
This change adds support for configuring the sampling period or frequency
for follower events.
Previously, only the group leader event could be configured to trigger
samples. With this change, follower events can also independently trigger
samples by specifying the 'period' or 'frequency' field in the config.
This aligns Perfetto's behavior more closely with `perf` semantics,
e.g., `perf record -e {context-switching,cpu-clock/period=<SAMPLE_PERIOD>}`,
and allows more flexible event sampling.
93a02bd to
6849261
Compare
|
@rsavitski
This keeps the given events in one group so they are read together as a consistent snapshot. The equivalent perfetto configuration will be: |
This change adds support for configuring the sampling period or frequency for follower events.
Previously, only the group leader event could be configured to trigger samples. With this change, follower events can also independently trigger samples by specifying the 'period' or 'frequency' field in the config.
This aligns Perfetto's behavior more closely with
perfsemantics, e.g.,perf record -e '{context-switches,cpu-clock/period=<SAMPLE_PERIOD>}', and allows more flexible PMU event sampling.