Add BatchLookup support for a pointer to a slice

[brycekahle]

BatchLookup claims it doesn't support a pointer to a slice, but sysenc.SyscallOutput does support pointers to slices:

ebpf/internal/sysenc/marshal.go

Lines 144 to 146 in 99a4e69

	// We're dealing with a pointer to a slice. Dereference and
	// handle it like a regular slice.
	value = value.Elem()

Could batchCount and sliceLen be updated to support pointer to slices?

[kwakubiney]

Could it be that it's the intended outcome? I read it as "keysOut" and "valuesOut" must be of type slice. A pointer to a slice or buffer will not work

[brycekahle]

You are right, I misread the comment.

That being said, the later functions do work with a pointer to a slice:

ebpf/internal/sysenc/marshal.go

Lines 144 to 146 in 99a4e69

	// We're dealing with a pointer to a slice. Dereference and
	// handle it like a regular slice.
	value = value.Elem()

[brycekahle]

Reworded to be a feature request

[brycekahle]

One of the motivations here is to avoid the 24 byte allocation necessary to pass the slice by value.

[lmb]

Can you add an example? The idea with BatchLookup is to use PossibleCPU() to create an appropriately sized slice and then reuse that for calls to the method. See #1271 though.

[brycekahle]

I think you are talking specifically about per-CPU, but my request applies even for normal maps.

[brycekahle]

Example:

// m is *ebpf.Map
keys := make([]uint32, 10)
values := make([]uint32, 10)

// current usage: allocates 24 bytes x 2 for the slice headers, but no more because of sysenc.SyscallOutput and unsafeBackingMemory
m.BatchLookup(keys, values, nil)

// proposed usage with no allocations
m.BatchLookup(&keys, &values, nil)

[lmb]

Okay, I guess that is because the slice headers need to go onto the heap since we turn them into any. Why are those allocations a problem for you? In my mind

ebpf/map_test.go

Lines 156 to 164 in 394be60

	lookupKeys := make([]uint32, n)
	lookupValues := make([]uint32, n*possibleCPU)
	var cursor BatchCursor
	var total int
	for {
	count, err = m.BatchLookup(&cursor, lookupKeys, lookupValues, nil)
	total += count
	if errors.Is(err, ErrKeyNotExist) {

will lead to two allocations per entire iteration of the map, which doesn't seem too bad to me. Maybe you can add an example?

The problem with pointer-to-slice for per-CPU maps is that they already have semantics for non-batch lookups. For legacy reasons they will cause the library to allocate the target slice. If we added pointer-to-slice for BatchLookup it would have to match those semantics so that we don't fully descend into madness (I already find it difficult to keep the rules straight). That in turn means it wouldn't be very useful for you due to additional allocations.

[arunsingh]

@lmb @brycekahle I can take a pass at this. I’m planning to keep it narrow: support pointer-to-slice for non-per-CPU BatchLookup / BatchLookupAndDelete, while leaving BatchUpdate, BatchDelete, and per-CPU batch semantics unchanged unless maintainers prefer otherwise. I will add lookup specific counter helper, use it only in batchLookup after per-CPU early return