internal/kconfig: Add possibility to filter kconfig file

[eiffel-fl]

Hi.

This commit introduces a new arguments to Parse(), it will be used to only add CONFIG_* corresponding to this map key to the returned map.

In this way, it permits reducing memory allocation, by only having CONFIG_* present in the eBPF code and not all the CONFIG_* set for the running kernel. This behavior mimics that of libbpf [1].

In terms of memory allocation, by filtering only with CONFIG_HZ, we reduced allocated bytes by approximately 79%:
BenchmarkParse-8 404 2484446 ns/op 1617336 B/op 9718 allocs/op
BenchmarkParseFiltered-8 751 1582447 ns/op 342272 B/op 9505 allocs/op

Best regards and thank you in advance for the review.

[lmb]

I think this is a good approach. I know that Timo wants to experiment with using a trie, but it's always better to avoid work rather than trying to optimize more.

There are some things we can do to speed this up some more, left a couple of comments.

[lmb]

I pushed a small commit which gets rid of some more allocations. I think that filtered kconfig is now plenty fast:

goos: linux
goarch: amd64
pkg: github.com/cilium/ebpf/internal/kconfig
cpu: 12th Gen Intel(R) Core(TM) i7-1260P
                 │  base.txt   │
                 │   sec/op    │
Parse-16           2.071m ± 3%
ParseFiltered-16   131.6µ ± 1%
geomean            522.1µ

                 │   base.txt   │
                 │     B/op     │
Parse-16           1.433Mi ± 0%
ParseFiltered-16   50.17Ki ± 0%
geomean            271.4Ki

                 │  base.txt   │
                 │  allocs/op  │
Parse-16           7.331k ± 0%
ParseFiltered-16    33.00 ± 0%
geomean             491.9

(NB: Filtered tests against CONFIG_HZ which is fairly high up in the file. Maybe we should choose the last item in config instead?)

@ti-mo are you OK with this approach?

[eiffel-fl]

Thank you for the approval and the commit but should I squash it with the first one?

(NB: Filtered tests against CONFIG_HZ which is fairly high up in the file. Maybe we should choose the last item in config instead?)

Sure! I can use CONFIG_ARCH_USE_MEMTEST instead!

[lmb]

should I squash it with the first one?

I'd keep it separate: it has an explanation in the commit message and it makes tracking of code provenance (signed-off-by) easier.

[eiffel-fl]

should I squash it with the first one?

I'd keep it separate: it has an explanation in the commit message and it makes tracking of code provenance (signed-off-by) easier.

Then, if we use CONFIG_ARCH_USE_MEMTEST, how should I handle this?
Pushing another commit? Editing one commit and its message?

[lmb]

Feel free to remove the exact benchmark results from my commit if that helps.

[eiffel-fl]

OK, I just modified a previous commit and did not touch yours.
Can you please point towards a documentation describing the compiler optimization for string()?

[lmb]

Here you go: golang/go#3512

[ti-mo]

Thanks! Left a few nits.

About the approach: I think we're generally more concerned about reducing allocs than reducing CPU usage. Creating garbage has cascading effects and is expensive to clean up, and a larger steady-state memory footprint (e.g. if we parse the whole kconfig and hold onto it) is problematic for many users, given the requests we've gotten for e.g. flushing vmlinux BTF etc. If we can avoid resident memory, let's do it. 🚀

[eiffel-fl]

Thank you for the review!
I normally addressed everything!

[lmb]

Please make sure to push formatted files: https://ebpf.semaphoreci.com/jobs/01c36378-27e1-413e-8f2d-d808a315f95e#L722

[eiffel-fl]

Please make sure to push formatted files: https://ebpf.semaphoreci.com/jobs/01c36378-27e1-413e-8f2d-d808a315f95e#L722

Sorry 😅 😅 😅!
Now it should do the trick!

[eiffel-fl]

Thank you for the merge and the reviews!

[lmb]

Thank you!