-
Notifications
You must be signed in to change notification settings - Fork 183
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: allow opting out of entry de-duplication #1733
Conversation
I would prefer this to be opt-out rather than opt-in, but if it is to be opt-in please make sure |
We can do opt-in instead! The main reason I started with it opted out is because I wanted to see what the code size impact was for globally disabling de-duplication; I'm not attached to this being the final form of the API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate that you've factored out the count type, so we could potentially use bigger or smaller counters in certain cases in the future.
Ringbuf entry de-duplication may help a lot of people fit more entries
in their ringbuffers, but it also generates a lot of
PartialEq
comparisons, so it's impossible to say if it's good or bad, actually.
Therefore, this branch adds a mechanism for code which does not record a
lot of duplicate entries to opt out of de-duplication by adding a
no_dedup
argument to theringbuf!
andcounted_ringbuf!
macros.Currently, I haven't actually changed any tasks to actually use this;
I'd prefer to do it in a follow-up PR. However, when de-duplication is
disabled globally, there's a noticable size difference for several
tasks, as seen below:
Ringbuf de-duplication enabled
Ringbuf de-duplication enabled