Makefile: build Cilium with madvdontneed=1

[aanm]

Setting this option makes Cilium to be compiled in a mode that will
allow it to reduce its process' RSS even if the system is not under
memory pressure.

By default, i.e. without 'madvdontneed' being set, Cilium will only
release its process' RSS when the system is under pressure. This
may be confusing for users since they will assume Cilium needs the
memory reported in RSS, which is not true.

Signed-off-by: André Martins andre@cilium.io

Make Go runtime to return unused memory to OS more often

Benchmark results:
100 node cluster with clusterloader2:
The red lines represent the beginning and end of the test

BASE VS NEW

[aanm]

test-me-please

[borkmann]

Do you have some before/after graphs for comparison?

[brb]

I think the release note is misleading. The env setting doesn't make the mem management more efficient. Most likely in opposite, as the Go runtime will go to OS more often to beg for memory.

[tklauser]

Unfortunately I think this won't have any effect on the compiled binary as GODEBUG is checked at runtime: https://github.com/golang/go/blob/682a1d2176b02337460aeede0ff9e49429525195/src/runtime/runtime1.go#L367

I think we would need to set GODEBUG=madvdontneed=1 in the cilium-runtime image instead.

FWIW, Fortunately, Go 1.16 will switch back to madvdontneed=1 as the default, see https://tip.golang.org/doc/go1.16#runtime:

On Linux, the runtime now defaults to releasing memory to the operating system promptly (using MADV_DONTNEED), rather than lazily when the operating system is under memory pressure (using MADV_FREE). This means process-level memory statistics like RSS will more accurately reflect the amount of physical memory being used by Go processes. Systems that are currently using GODEBUG=madvdontneed=1 to improve memory monitoring behavior no longer need to set this environment variable.

[aanm]

test-me-please

[aanm]

test-me-please

[aanm]

retest-gke

[aanm]

I'll try to run some tests before merging to have some idea.

[aanm]

test-me-please

[kkourt]

I think this change makes sense.

While it is not clear to me whether the test numbers added by @aanm actually show the added CPU cost of MADV_FREE vs MADV_DONTNEED, I believe that this change is the right way to move forward since it's going to be adopted in 1.16 anyway, and it also solves the practical issue of better reporting of memory usage.

From the description of the change: https://go-review.googlesource.com/c/go/+/267100/, it also seems to me that the go scavenger can pace itself now to reduce the effect whatever added overhead.

I think we should go with the future default sooner rather than later, and if we came into issues we can always use madvdontneed=0.

[rolinh]

Could we prepend the notes to remove the ENV with a FIXME so that they are easy to find/grep?

[brb]

@aanm you might want to still address this #14634 (comment).

[aanm]

the CI was green, I just applied this suggestion #14634 (review) merging