perf.Reader Slow events can't be read if Watermark was set, even with deadline timer.

[albe19029]

In situations when we have high speed events producer, and high speed events consumer ReadInto call pr.poller.Wait function for every event. In this case our thread sleep and wakeup very often. And in our tests it consumes a lot of CPU (15%)

While creating perf.Reader it is also possible to set Watermark option, but it working bad - as it only set EPOLLIN to file descriptor when buffer will be full, and on low events count - this can be blocked for a very long time.

What is possible is to return ErrEOR if wait parameter is false. In this case we can deсide what to do. For example use ratelimit.Limiter to limit polls count per second.

ebpf.patch

[albe19029]

pollLimiter := ratelimit.New(20)

// try to process collected events first
err := r.reader.ReadInto(&r.record, perf.NoWait)
if err == perf.ErrEOR {
// now wait for new events, but limit amount of polls per second
pollLimiter.Take()
err = r.reader.ReadInto(&r.record, perf.Wait)
}

This is how we limit to no more then 20 waits per second with patched code. The CPU drops to 3-5%

[florianl]

Hi @albe19029
And thanks for your feedback!
With #1404 there is a PR with the potential to improve your experience.

[albe19029]

Sorry, I can't understand how this can improve experience. Watermark set limit in bytes, WakeupEvents in samples. The problem is the same - we can't control how much can we wait in time. As both this events will have notification from kernel when amount of bytes or samples will be exceeded. If there will be low events count - time can be from nanoseconds to seconds. We can't set that we want: either time has expired or the number of events has been exceeded. With the patch we want to overcome the linux kernel limitation. To wait not on poll operation, but on our user space wait operation (pollLimiter.Take())

[albe19029]

By the way there is a deadline timer for poller.Wait operation which can be set with SetDeadline.
But when code wake up by this timer it only works with nEvents (even if there is already data in rings). Is it possible to check how much data in each ring buffer, and read it even without epoll notification?
In this case we will be notified only if n events are in buffer, or time has elapsed and we need to read events that are in buffer?

[albe19029]

As I see from linux kernel source:
https://elixir.bootlin.com/linux/latest/source/kernel/events/core.c#L7485

events count will not be changed if we will read some data in parallel, and we will be notified every n events, not when there will be less data in buffer then n events or bytes. But I think it is ok (or need to fix it in kernel if for someone it is critical).

[albe19029]

ebpf_v2.patch

Can you please take a look at this patch. I checked only for Overwritable=false option, so the code can be different for this case.
But Overwritable=false and Watermark option = 1/16 of perCPUBuffer, on my project everything is working correct with better perfomance.

[florianl]

Being required to handle events from perf event ringbuffer with a high variance in the number of events to handle over time is always a challenge.
Before going on, can you share some insights in the experiments you did using different value combinations for (*Reader)SetDeadline() and ReaderOptions.Watermark? How did these numbers change, when experimenting with #1404? Can you provide these numbers?

[albe19029]

So what I do in my test - just monitor kprobe/vfs_open to get path of each opened file. If I use cilium/ebpf with version 0.13.2 I get next CPU result with this script:
script.txt

My ring buffer size for each CPU 512k
I use top -p :
without patch:
Watermark 1, deadline - forever - 7.2 - 9.2% of CPU

I can't set watermark bigger as I need prediction in time, if there are events in ring buffers:

with patch:
Watermark 128k, deadline 100 ms - 1.3 - 1.7% of CPU
Watermark 128k, deadline 50 ms - 2.0 - 2.3 % of CPU

I didn't checked with #1404 as it has the same limitation as Watermark, and I can set it only to 1. Think the performance will be the same.

[albe19029]

Do you need working benchmark? If so, please tell me - will need some time to prepare simple benchmark test.

[florianl]

I read the following as the baseline:

without patch:
Watermark 1, deadline - forever - 7.2 - 9.2% of CPU

What I'm missing here are tests with different value combinations for watermark and deadline without your patch. e.g.

Watermark	Deadline	CPU
1	0 ms	7.2 - 9.2%
1	10 ms
1	50 ms
1	100 ms
96	0 ms
96	10 ms
96	50 ms
96	100 ms
512	0 ms
512	10 ms
512	50 ms
512	100 ms

From my point of view, the given baseline is hardly comparable with the given numbers for when a patch is applied.

[albe19029]

If I set watermark and deadline - performance will be the same with and without my patch.
My patch only fix worst case with low events count. On low events count with Watermark = 128 Kb it it may take a few minutes before I get events. Even if I will set deadline timer - the code will not read data from ring buffer until there will be notification from epoll.

[albe19029]

And after a few minutes the process may no longer exist

[albe19029]

For now I can't set watermark more then 1 to have result in predictable time. And this watermark 1 harms the performance if there are a lot events.

[albe19029]

Is it clear what situation I want to fix?

[albe19029]

@florianl Is my comment was unclear? Do we need watermark and deadline results without patch to continue?

[albe19029]

Good day. Just wanted to ask if our problem will be fixed somehow?

Just to summarize:
we want our code work with good performance both with low events count per second generation, and frequent events count generation.

So to work with frequent events we had to set Watermark option from 1 to 128K. With that we reduce CPU from 7.2 - 9.2% to 1.7% - 2.0%.
But setting this option will block low events - as until ring buffer size will be less then watermark size - we will not get events, even if they are already in CPU rings. This wait can be from seconds to minutes. And we can't get them, even if we set deadline timer with SetDeadline. As code only read ring data after epoll notification.

Good fix is to add some timer (maybe in ReaderOptions option) per cpu ring, and check each ring if time has elapsed and we don't have notification from epoll.

Good, but not perfect is proposed ebpf_v2.patch , but fast single ring will also don't allow read slow rings in worst case.

Hope it is clear that problem exists and it is possible to fix it somehow.
Thanks.

[lmb]

In ringbuf we have a code path which does check the ring for contents even if there was no wakeup:

ebpf/ringbuf/reader.go

Lines 150 to 154 in 9c1d099

	if errors.Is(err, os.ErrDeadlineExceeded) && !r.ring.isEmpty() {
	// Ignoring this for reading a valid entry after timeout
	// This can occur if the producer submitted to the ring buffer with BPF_RB_NO_WAKEUP
	err = nil
	}

My suggestion would be to implement something similar for perf and then SetDeadline() would work as you desire.

[albe19029]

what about single fast ring problem? In this case there will be no ErrDeadlineExceeded (epoll will notify this ring), and other rings will not be read anyway?

[albe19029]

ringbuffer has a single ring - so there are no such a problem. Forgot about it.

[brycekahle]

I believe we (Datadog) want something similar. We want to set our Watermark (or WakeupEvents) higher to reduce userspace wakeup frequency and CPU usage, but we also want a way to read events from a perf buffer before the watermark has been reached. This allows us to ensure we see all events within a specific timeframe.

One challenge already mentioned is one (or more) highly active CPU perf buffers can prevent us from seeing the other CPU perf buffers, because they will always be ready. We essentially need a way to ensure all CPU perf buffers are read. This likely means an API other than Read is needed for this use case.

[albe19029]

I don't know the exact procedure in your project. But I wanted to ask whether this proposal will be accepted for improvements. And what is needed to speed up decision making. Is it possible to implement patch?
I apologize in advance if I am too annoying.

[florianl]

There is currently a discussion ongoing on Slack, how things can be improved. As a result your concern should be addressed, while it should not break things existing users of the package.