perf-event icon indicating copy to clipboard operation
perf-event copied to clipboard

Replace `Event` enum with `Builder` methods

Open jimblandy opened this issue 2 years ago • 1 comments

Maybe we should ditch the idea of an Event value, and simply have hardware, software, cache, and breakpoint methods directly on Builder. Whatever fields the Event enum variants had would become arguments to those methods.

What's important to preserve here is the property that you can't select an event kind without providing all the necessary information to get a meaningful result - like, say, a breakpoint without an address. If we just let people set perf_event_attr fields directly, that would be a risk. But by making people produce an Event the idea was that you'd make sure they'd filled everything in.

Replacing each Event variant with a method on Builder could accomplish the same goal, and remove the pointless indirection of "first we build the Event; then we wait for them to hand it to us (which is the only thing you can do with an Event anyway); then we move the fields into the perf_event_attr struct." Instead, you'd just have the one Builder method that just does the deed.

jimblandy avatar Nov 11 '22 21:11 jimblandy

As a user of the API, if you do this, please at least ensure that there is still a single method that can be used with all the event types. Being able to call builder.kind(<literally any event type>) is really quite nice and it would be a shame if that went away. Whether that happens through impl Into<Event> or another trait that's implemented for all events doesn't really matter.

Phantomical avatar Nov 11 '22 23:11 Phantomical