-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
profiling: higher memory sampling rate #11308
Conversation
cmd/podman/root.go
Outdated
} | ||
} | ||
if cmd.Flag("memory-profile").Changed { | ||
// Same value as the default in github.com/pkg/profile. |
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.
Shouldn't there be a similar os.Create(cfs.MemoryProfile) be here as well?
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.
That's happening below in the pre-stop hook. The CPU profile is created at the beginning an continuously being written to while the memory profile is created at the very end with all samples.
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.
What happens if the file can not be created? I would prefer to know early rather then run my entire test and have it blow up in the end?
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.
It'll blow up at the end. There's not much we can do, since the memory profile must be written at the end. It's just how the stdlib wants it.
Note that the flag is hidden and meant to be only used by the devs.
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.
Ok.
LGTM
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Increase the memory-sampling rate to the same default as github.com/pkg/profile. Allow for custom rates by reading the `MemProfileRate` env variable. [NO TESTS NEEDED] since it's a dev only flag and not part of CI. Signed-off-by: Valentin Rothberg <[email protected]>
/hold cancel |
Increase the memory-sampling rate to the same default as
github.com/pkg/profile. Allow for custom rates by reading
the
MemProfileRate
env variable.Signed-off-by: Valentin Rothberg [email protected]
@mheon @baude PTAL