jimblandy / perf-event

perf-event: a Rust interface to Linux performance monitoring
Apache License 2.0
111 stars 19 forks source link

Builder should just construct a perf_event_attr struct as it goes along #6

Closed jimblandy closed 4 years ago

jimblandy commented 4 years ago

At present, Builder has its own representation of the configuration selected by the methods called on it, and it uses that representation to initialize a perf_event_attr struct to pass to perf_event_open when build is finally called.

It would be better to simply have Builder contain a perf_event_attr struct, and let its methods initialize its fields. It could continue to take the group and cgroup file descriptors by reference, but its lifetime parameter could be used by a PhantomData<&File>, while the raw file descriptor gets copied into the perf_event_attr struct. This would hold the Files frozen until we had a chance to construct the perf fd.

The structure size could be initialized to some older version, and updated with max as newer parts of the structure were initialized, to maximize compatibility with older kernels.

jimblandy commented 4 years ago

The cgroup and group are parameters to perf_event_open, not fields of perf_event_attr, so there's not much simplification possible there. Those have to be separate fields of Builder anyway, and it's simpler and more well-typed to do it the way it's done now.

The kernel API copes with perf_event_attr structs both smaller (older) and larger (newer) than the kernel expects, so it's not necessary to play games with the size.