ebpf icon indicating copy to clipboard operation
ebpf copied to clipboard

sysenc: dealing with `[]struct{ ... }` forces allocations

Open lmb opened this issue 1 year ago • 0 comments

We currently inherit an allocation problem from encoding/binary: binary.Size doesn't cache it's result for slices of structs. This means that our zero-alloc code path via sysenc isn't zero-alloc, since we end up hitting an allocation in reflect.

A benchmark to illustrate:

$ go test -run XX -bench 'BenchmarkUnmarshal/sysenc\.explicitPad' ./internal/sysenc/
goos: linux
goarch: amd64
pkg: github.com/cilium/ebpf/internal/sysenc
cpu: 12th Gen Intel(R) Core(TM) i7-1260P
BenchmarkUnmarshal/*sysenc.explicitPad-16         	25056632	       44.09 ns/op	      0 B/op	      0 allocs/op
BenchmarkUnmarshal/[]sysenc.explicitPad-16        	18953799	       64.01 ns/op	      8 B/op	      1 allocs/op
BenchmarkUnmarshal/[]sysenc.explicitPad#01-16     	18458588	       65.54 ns/op	      8 B/op	      1 allocs/op
BenchmarkUnmarshal/[]sysenc.explicitPad#02-16     	18186916	       67.05 ns/op	      8 B/op	      1 allocs/op
PASS
ok  	github.com/cilium/ebpf/internal/sysenc	5.075s

Going from a single explicitPad to a slice of them causes an allocation. The allocation happens in binary.dataSize:

Total: 152MB
ROUTINE ======================== encoding/binary.Size in /usr/local/go/src/encoding/binary/binary.go
         0      152MB (flat, cum)   100% of Total
         .          .    466:func Size(v any) int {
         .      152MB    467:	return dataSize(reflect.Indirect(reflect.ValueOf(v)))
         .          .    468:}
         .          .    469:
         .          .    470:var structSize sync.Map // map[reflect.Type]int
         .          .    471:
         .          .    472:// dataSize returns the number of bytes the actual data represented by v occupies in memory.
ROUTINE ======================== encoding/binary.dataSize in /usr/local/go/src/encoding/binary/binary.go
         0      152MB (flat, cum)   100% of Total
         .          .    476:func dataSize(v reflect.Value) int {
         .          .    477:	switch v.Kind() {
         .          .    478:	case reflect.Slice:
         .      152MB    479:		if s := sizeof(v.Type().Elem()); s >= 0 {
         .          .    480:			return s * v.Len()
         .          .    481:		}
         .          .    482:
         .          .    483:	case reflect.Struct:
         .          .    484:		t := v.Type()

In the past I've fixed the same problem for plain structs using a cache: https://github.com/golang/go/commit/c9d89f6bacd66d4765cf36d2a4b121392921c5ed I didn't take slices of structs into account since they are kind of esoteric. Turns out we now need to encode those when doing zero-alloc batch lookups, see #1254

One option is to fix this upstream, but it'll take a long time for this to reach us. I think we should fix upstream and in addition copy the implementation of binary.Size and fix the problem in our copy.

lmb avatar Dec 05 '23 17:12 lmb