perf: flush Record when deadline is exceeded#1429
Merged
lmb merged 4 commits intocilium:mainfrom May 10, 2024
Merged
Conversation
albe19029
reviewed
Apr 11, 2024
dbbce35 to
9f1ded5
Compare
9f1ded5 to
26aa88a
Compare
Contributor
Author
|
Reworked a little bit and fixed test failures. PTAL. |
florianl
approved these changes
May 10, 2024
Signed-off-by: Lorenz Bauer <[email protected]>
eventFd.add hardcoded the value to add to 1 instead of using the argument. This hasn't caused a bug since we never call add with a different value. Signed-off-by: Lorenz Bauer <[email protected]>
Waking up user space from the kernel is expensive and so the perf reader allows adjusting the rate at which wakeups happen. This saves CPU at the cost of latency: some data will remain in the buffer for longer. The reader is an abstraction over multiple ring buffers. It only reads from a ring buffer if it has received a wakeup from the kernel. This is wasteful because wakeups are expensive (due to context switching and so on) but checking a ring for contents is cheap (just an atomic load). Change the behaviour so that we read data from any ready ring buffer regardless of why we were woken up. Signed-off-by: Lorenz Bauer <[email protected]>
Watermark and WakeupEvents allow reducing the rate at which user space is woken up. This creates an edge case where Read() returns os.ErrDeadlineExceeded even though there is data in one of the rings. This is also a problem in the ringbuf package, where we've solved this by checking whether there is any data when encountering an ErrDeadlineExceeded. Adopt the same approach for the perf reader. TestPerfReaderWakeupEvents has to change since it relies on the old behaviour of not checking for pending data. Rework the test so that it doesn't test the ErrDeadlineExceeded error: it's not so important that we don't wake up early. Signed-off-by: Lorenz Bauer <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
epoll: simplify timeout calculation
epoll: fix eventFd.add
perf: opportunistically check all rings for data
perf: flush Record when deadline is exceeded