-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[agent-smith] Reduce cpu and memory consumption #10356
Conversation
if err != nil { | ||
return false, xerrors.Errorf("cannot read stream head: %w", err) | ||
} | ||
in.header = head |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cache at first.
if err != nil { | ||
return false, err | ||
} | ||
in.rodata = rodata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cache at first.
if len(in.rodata) > 0 { | ||
rodata = in.rodata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a cache 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense
if len(in.header) > 0 { | ||
head = in.header |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a cache
var symbols []string | ||
if len(in.symbols) > 0 { | ||
symbols = in.symbols | ||
} else { | ||
executable, err := elf.NewFile(in.Reader) | ||
if err != nil { | ||
return false, xerrors.Errorf("cannot anaylse ELF file: %w", err) | ||
} | ||
|
||
symbols, err := ExtractELFSymbols(executable) | ||
if err != nil { | ||
return false, err | ||
symbols, err = ExtractELFSymbols(executable) | ||
if err != nil { | ||
return false, err | ||
} | ||
in.symbols = symbols |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
symbols := make([]string, len(syms)+len(dynsyms)) | ||
i := 0 | ||
for _, s := range syms { | ||
symbols[i] = s.Name | ||
i += 1 | ||
} | ||
|
||
for _, s := range dynsyms { | ||
symbols = append(symbols, s.Name) | ||
symbols[i] = s.Name | ||
i += 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these refactoring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We know the number of elements, so I allocate the underlying array once instead of appending to the slice which includes resizing i.e. reallocation of the array.
@Furisto I'd like to know how you profile the agent-smith? |
https://www.notion.so/gitpod/Profiling-Go-Code-a0de0ddcb34a48ac9ebee7fa4f0faf52 |
Description
We have cpu and memory spikes in agent-smith, with big contributors being reading the elf data of binaries. This data is reread for every signature that has been defined. With this change we cache the data for the duration of the match phase.
Related Issue(s)
Fixes ##10331
How to test
Profile agent-smith in preview environment
Release Notes