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

Improve performance of BPF map buffer writes #1312

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

javierhonduco
Copy link
Contributor

@javierhonduco javierhonduco commented Feb 7, 2023

Using binary.Write to write to memory buffers we use to update BPF maps is not only using quite a bit of CPU cycles but also incurs quite a bit of small allocations. In particular, every single write performs an allocation.

This commit reduces allocations to 0 and CPU cycles by ~14x by writing to the buffer without performing any allocations. We do this with a new type, defined over a byte slice, which directly writes to the buffer using the lower-level binary APIs.

This also introduces two other advantages:

  • The underlying array backing the slice will never grow in case of bugs. We want the buffers to be static and of exactly the size we expect in BPF.
  • We can zero the buffers more efficiently. We need to do this to ensure that there is no stale information in the buffers we reuse.

This new type has to be used with care as we expect the Slice method to request a size equal or smaller to the available capacity.

Test Plan

Added unit tests, ran locally for a while with no issues and binary.Write barely showing up in the profiles. The new methods don't take much CPU either.

$ go test -v ./pkg/profiler/ -bench=. -count=30 > bench.txt
$ benchstat bench.txt
name                          time/op
EfficientBufferSliceWrite-12  2.18ns ± 4%
BinaryWrite-12                31.3ns ±26%

name                          alloc/op
EfficientBufferSliceWrite-12   8.00B ± 0%
BinaryWrite-12                 16.0B ± 0%

name                          allocs/op
EfficientBufferSliceWrite-12    0.00
BinaryWrite-12                  1.00 ± 0%

Part of #1286

Signed-off-by: Francisco Javier Honduvilla Coto javierhonduco@gmail.com

Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

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

I think I would have personally wrapped the byte slice in a struct and kept a current index instead (I just find that a little easier to reason about), but this works perfectly well!

pkg/profiler/buffer.go Show resolved Hide resolved
@javierhonduco javierhonduco force-pushed the efficient-maps-writing branch from df2cff0 to dda31b7 Compare February 8, 2023 08:20
@javierhonduco javierhonduco marked this pull request as ready for review February 8, 2023 08:20
@javierhonduco javierhonduco requested a review from a team as a code owner February 8, 2023 08:20
@javierhonduco javierhonduco force-pushed the efficient-maps-writing branch from dda31b7 to 0916278 Compare February 8, 2023 08:29
Using `binary.Write` to write to memory buffers we use to update BPF
maps is not only using quite a bit of CPU cycles but also incurs in quite
a bit of small allocations. In particular, every single write performs
an allocation [0].

This commit reduces allocations to 0 and CPU cycles by ~14x by writing to
the buffer without performing any allocations. We do this with a new
type, defined over a byte slice, which directly writes to the buffer
using the lower level `binary` APIs.

This also introduces two other advantages:
- The underlying array backing the slice will never grow in case of
  bugs. We want the buffers to be static and of exactly the size we
expect in BPF.
- We can zero the buffers more efficiently. We need to do this to ensure
  that there is not stale information in the buffers we reuse.

This new type has to be used with care as we expect the `Slice` method
to request a size equal or smaller to the available capacity.

Test Plan
=========

Added unit-tests, ran locally for a while with no issues and
`binary.Write` barely showing up in the profiles. The new methods don't
take much CPU either.

```
$ go test -v ./pkg/profiler/ -bench=. -count=30 > bench.txt
```

```
$ benchstat bench.txt
name                          time/op
EfficientBufferSliceWrite-12  2.18ns ± 4%
BinaryWrite-12                31.3ns ±26%

name                          alloc/op
EfficientBufferSliceWrite-12   8.00B ± 0%
BinaryWrite-12                 16.0B ± 0%

name                          allocs/op
EfficientBufferSliceWrite-12    0.00
BinaryWrite-12                  1.00 ± 0%
```

[0]: https://github.com/golang/go/blob/02d8ebda8398342516a24a3e8435d527ed156cab/src/encoding/binary/binary.go#L345
Signed-off-by: Francisco Javier Honduvilla Coto <javierhonduco@gmail.com>
@javierhonduco javierhonduco force-pushed the efficient-maps-writing branch from 0916278 to 14ee01c Compare February 8, 2023 08:29
@javierhonduco javierhonduco merged commit ae45c86 into main Feb 8, 2023
@javierhonduco javierhonduco deleted the efficient-maps-writing branch February 8, 2023 08:48
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

❤️ 💚 💙 💛 💜

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.

3 participants