-
Notifications
You must be signed in to change notification settings - Fork 443
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
ddtrace/tracer: Memoize the PID #438
Conversation
The os.Getpid() call is visible in profile flamegraphs of busy servers. Memoize it as the PID won't change in the lifetime of a Go process.
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.
Found some fixes!
P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).
Co-Authored-By: fixmie[bot] <fixmie[bot]@users.noreply.github.com>
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.
Makes sense, looks good and passes the existing tests related to the pid
tag.
Thanks for providing this optimization. Could you take a look at the contribution guidelines, and in the future, consider opening an issue to discuss changes first. @gbbr any action required for the Fixmie error, or is it a CI flake? |
Thank you both! This is a good change! |
Unsure about CI. It says "Internal error" 😴 |
@CAFxX would you mind helping out the |
Commented |
The os.Getpid() call is visible in profile flamegraphs of busy servers. Memoize it as the PID won't change in the lifetime of a Go process.
The os.Getpid() call is visible in profile flamegraphs of busy servers.
Memoize it as the PID won't change in the lifetime of a Go process.