-
Notifications
You must be signed in to change notification settings - Fork 166
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
ringbuf: Add event counters #1621
Commits on Feb 22, 2024
-
ringbuf: Add event counters (#1621)
Ringbufs provide a fixed-size buffer for storing diagnostic events, which may include data. This is very useful for debugging. However, because the ring buffer has a fixed size, only the last `N` events are visible at any given time. If a ringbuf has a capacity of, say, 16 events, and a given event has occurred 10,000 times over the lifespan of the task, there's currently no way to know that the event has happened more than 16 times. Furthermore, there's no way to use the ringbuf to determine whether a given event has _ever_ occurred during the task's lifetime: if a given variant is not _currently_ present in the ringbuf, it may still have occurred previously and been "drowned out" by subsequent events. Finally, some targets disable the ringbuf entirely, for space reasons, and on those targets, it may still be desirable to be able to get some less granular diagnostic data out of the ringbuf events a task records. Therefore, this commit extends the `ringbuf` crate to support generating counts of the number of times a particular event variant has been recorded, in addition to a fixed-size buffer of events. These counters provide less detailed data than inspecting the ringbuf contents, because they don't record the values of any _fields_ on that event. However, they allow recording very large numbers of events in a fixed amount of space, and provide historical data for events that may have occurred in the past and then "fallen off" the end of the ringbuf as new events were recorded. By inspecting these counters, we can determine if a given variant has *ever* been recorded, even if it isn't currently in the buffer, and we can see the total number of times it has been recorded over the task's entire lifetime. Event counters are implemented as an array of `AtomicUsize` counts paired with an `&'static str` name for the variant that was recorded. A new `counted_ringbuf!` macro is provided to create a ringbuf and also generate a `{RINGBUF_NAME}_COUNTS` static containing its counters. In order to produce event counts, a value type must implement a new `ringbuf::Event` trait, which allows enumerating the variant names of an `enum` type and assigning indices into the counter array to each variant. A `#[derive(ringbuf::Event)]` macro allows generating implementations of this trait automatically. Finally, a new `event!` macro in the `ringbuf` crate both records a ringbuf entry _and_ increments the corresponding event count. I chose to add new macros rather than modifying the existing `ringbuf!` macros to allow the event counters to be adopted incrementally; otherwise, it would be necessary to add `#[derive(ringbuf::Event)]` to _every_ type that's ever recorded as a ringbuf evntry as part of this commit. Because recording event counters requires only `8 * <N_VARIANTS>` bytes, counters are currently recorded even when the `ringbuf` crate has the `disabled` feature set. This way, we can still record some diagnostic data on targets that don't have the space to store the ringbuf itself. If it becomes necessary to also disable counters, we could add a separate `disable-counters` feature as well, so that a target can pick and choose between recording ringbuf entries, counts, or both. Future work will include: - [ ] Actually adopting event counters in existing ringbufs. - [ ] Updating `humility ringbuf` to also dump ringbuf event counts.
Configuration menu - View commit details
-
Copy full SHA for 7791354 - Browse repository at this point
Copy the full SHA 7791354View commit details -
gimlet-seq: Count ringbuf events (#1621)
This commit changes `gimlet-seq-server` to use the `counted_ringbuf!` macro added in e3b942c
Configuration menu - View commit details
-
Copy full SHA for 3504b41 - Browse repository at this point
Copy the full SHA 3504b41View commit details -
ringbuf: generate structs for event counters (#1621)
As suggested by @cbiffle, this commit changes the ringbuf event counters code to generate a struct with named fields, rather than an array. This presents a nicer API to debuggers, and avoids the need to stick a bunch of string constants in the binary.
Configuration menu - View commit details
-
Copy full SHA for 8b8b7a1 - Browse repository at this point
Copy the full SHA 8b8b7a1View commit details -
Configuration menu - View commit details
-
Copy full SHA for 8ab3ae6 - Browse repository at this point
Copy the full SHA 8ab3ae6View commit details -
ringbuf: get rid of separate
count_entry!
macro (#1621)I didn't love that there were separate `count_entry!` and `ringbuf_entry!` macros --- it would be nicer if adopting event counts didn't require changing every macro invocation. This change reworks the macros to use a single `ringbuf_entry!` macro, and dispatch whether or not to count events based on whether the ringbuf was *declared* using `counted_ringbuf!` or `ringbuf!`. The way this works is by generating a module with the same name as the ringbuf, and generating a function inside that module that actually records the entry (and may or may not record event counts, depending on the macro that declared the ringbuf). This way, `ringbuf_entry!` can simply call `<RINGBUF_NAME>::record(...)`, and the function will determine whether or not to count events. It occurs to me that we could also use the module to generate the event counter static without having to do a proc-macro-y concatenation of idents.
Configuration menu - View commit details
-
Copy full SHA for f046ba0 - Browse repository at this point
Copy the full SHA f046ba0View commit details -
ringbuf: remove
concat_idents!
proc-macros (#1621)Since the previous commit changes the `ringbuf!` and `counted_ringbuf!` macros to generate a module for each ringbuf, we can stick the counters static in there. This obviates the need to use proc-macros for incrementing the counters just so we can concatenate the ringbuf's name with `_COUNTS` to determine the name of the counter static. Now, we only use the proc macro for deriving `Count`. This allows us to make the derive macro optional.
Configuration menu - View commit details
-
Copy full SHA for 3714c19 - Browse repository at this point
Copy the full SHA 3714c19View commit details -
fix example breaking with
cargo test --workspace
Currently, the `cargo test --workspace` invocation on CI tries to build example code for the host target, which is nice, as ensuring that the macro example builds at all is a useful test of the macros. However, because `ringbuf` depends on crates that currently only build for ARM targets, building the example for x86 breaks CI. To fix this, I've changed the `build_util::expose_m_profile()` function to return a `Result`, which is ignored when building lib crates like `armv6m-atomic-hack` and `static-cell` on non-ARM targets, and `unwrap`ped when building actual Hubris tasks. This way, it's now possible to build the lib crates for the host target, so the example compiles, but trying to compile Hubris for x86 will still fail (correctly).
Configuration menu - View commit details
-
Copy full SHA for d6c275b - Browse repository at this point
Copy the full SHA d6c275bView commit details -
ringbuf: dispatch counting using a trait instead
I think this is my nicest design thus far, as it avoids a lot of the problems with the submodule based approach, and also only requires the proc macro if you want to derive counters.
Configuration menu - View commit details
-
Copy full SHA for 474239b - Browse repository at this point
Copy the full SHA 474239bView commit details -
Configuration menu - View commit details
-
Copy full SHA for 6c02a7b - Browse repository at this point
Copy the full SHA 6c02a7bView commit details -
Configuration menu - View commit details
-
Copy full SHA for 71e6013 - Browse repository at this point
Copy the full SHA 71e6013View commit details -
don't derive
Debug
for counter typesThis should hopefully help shave off a few bytes of `.rodata`?
Configuration menu - View commit details
-
Copy full SHA for f10abb6 - Browse repository at this point
Copy the full SHA f10abb6View commit details -
Configuration menu - View commit details
-
Copy full SHA for 82771e3 - Browse repository at this point
Copy the full SHA 82771e3View commit details -
Configuration menu - View commit details
-
Copy full SHA for a785972 - Browse repository at this point
Copy the full SHA a785972View commit details -
Configuration menu - View commit details
-
Copy full SHA for 2379114 - Browse repository at this point
Copy the full SHA 2379114View commit details
Commits on Feb 23, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 51db96b - Browse repository at this point
Copy the full SHA 51db96bView commit details -
Configuration menu - View commit details
-
Copy full SHA for 2d41888 - Browse repository at this point
Copy the full SHA 2d41888View commit details -
Configuration menu - View commit details
-
Copy full SHA for adeba16 - Browse repository at this point
Copy the full SHA adeba16View commit details -
Configuration menu - View commit details
-
Copy full SHA for d5f8d5c - Browse repository at this point
Copy the full SHA d5f8d5cView commit details