Skip to content
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

perf_event: Remove the size limit from EventIter. #30

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

ishitatsuyuki
Copy link
Contributor

To sort perf events on the fly, all available events from one stream must be examined at once, which is not possible with the current EventIter restricted to up to 31 event at once.

Rewrite the out-of-order commit logic so that pending commits are tracked in a binary heap, which will remain small as long as events are dropped in-order but can also comfortably accommodate an unbounded amount of out-of-order drop.

This changes the event iteration order. Previously, we iterated over 31 events from each buffer before going on to the next. Now, all available events in each buffer will be consumed before going on to the next. This incurs more buffering than before, but live event sorting will incur even more buffering, so seeing this as a transition step it should be acceptable.

To sort perf events on the fly, all available events from one stream must be
examined at once, which is not possible with the current EventIter restricted
to up to 31 event at once.

Rewrite the out-of-order commit logic so that pending commits are tracked in
a binary heap, which will remain small as long as events are dropped in-order
but can also comfortably accommodate an unbounded amount of out-of-order drop.

This changes the event iteration order. Previously, we iterated over 31 events
from each buffer before going on to the next. Now, all available events in
each buffer will be consumed before going on to the next. This incurs more
buffering than before, but live event sorting will incur even more buffering,
so seeing this as a transition step it should be acceptable.
Copy link
Owner

@mstange mstange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I hadn't fully understood the old code or what it was doing with the bitmask, but the new behavior is much easier to understand in my opinion.

Thank you for this great work.

@mstange mstange merged commit a9bcf4f into mstange:main Feb 27, 2023
@ishitatsuyuki ishitatsuyuki deleted the event-iter branch February 28, 2023 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants