Skip to content
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

argumentParsers: precompile argument and string maps #130

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

NDStrahilevitz
Copy link
Contributor

The Fix

Many String() and Parse functions had inner constant helper maps.
These maps were reallocated on each call, which could overtime cause signifacnt GC overhead.
This fix initializes the maps before the functions, making sure they are reused and not repeatedly reallocated.

Performance analysis

This fix was found while benchmarking tracee with a specific workload - a script running strace ps -ef repeatedly.

Running pprof on this case gave the following flame graph:

image

Note the runtime.mapassign_fast64 that is a significant repeating code path.

With this fix we get this flame graph:

image

Simply preallocating the map one time on the module init avoids the constant repeating calls to runtime.mapassign_fast64 while constructing the map on each function call.

Of course, this workload brought out this function on the extreme due to it being relevant to the workload running in a loop, however it isn't impossible that these helpers would cause overhead overtime for different type of workloads (There are many more maps there were optimized in this PR).

This is a quick and easy change which is similar in nature to precompiling a regex.

Many String() and Parse functions had inner constant helper maps.
These maps were reallocated on each call, which could overtime cause signifacnt GC overhead.
This fix initializes the maps before the functions, making sure they are reused and not repeatedly reallocated.
Copy link
Contributor

@rafaeldtinoco rafaeldtinoco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM AND it is a nice change indeed. I'm fixing the git log (to wrap at correct column). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants