[networks] Improve memory allocation pattern #9206
Conversation
There was a problem hiding this comment.
Not something for this PR, but I'd be curious if we get any perf gains from getting rid of the PerfHandler (and thus the channel) and using the callback directly. We might need to think about any thread safety concerns, but I know channels can be heavy-handed.
There was a problem hiding this comment.
Good point. I was also wondering if we could write our own perf poller or add an alternative implementation to the ebpf library that uses a buffer pool, because a large chunk of the memory allocations now come from it.
p-lambert
force-pushed
the
pedro/tracer-mem-improvements
branch
from
4f4fc54 to
77971fa
Compare
p-lambert
added
changelog/no-changelog
| } | ||
| err = ebpfTracer.Start(tr.closedCB) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not start ebpf manager: %s", err) |
There was a problem hiding this comment.
Do we need to tear anything down at this point, since the tracer object has been created along with possible HTTP and DNS features? I think this is why it was before object creation before.
There was a problem hiding this comment.
Great catch. I added some changes to handle that scenario
brycekahle
left a comment
brycekahle
left a comment
There was a problem hiding this comment.
One comment, but otherwise looks good.
2 participants
What does this PR do?
This PR refactors some parts of the network-tracer to reduce the number and the bursty pattern of allocations.
In our load test server I witnessed a ~30% drop in bytes allocated/s and a ~20% CPU drop
bytes allocated/s
CPU cores
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Describe how to test your changes
I'm only changing implementation details and everything should be covered by tests already.
Checklist
changelog/no-changeloglabel has been applied.need-change/operatorandneed-change/helmlabels has been applied if applicable.team/..label has been applied, if known.Triagemilestone is set.Note: Adding GitHub labels is only possible for contributors with write access.