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

Add profiling to help with OOM/performance investigations #207

Merged
merged 3 commits into from
May 10, 2024

Conversation

egibs
Copy link
Member

@egibs egibs commented May 10, 2024

This is a quick PR to help with identifying the cause(s) of #204 which adds CPU and Memory profiling along with tracing.

If anything, I wanted the code to be available to aid with the investigation -- this doesn't necessarily need to be merged; however, as the number of capabilities grows it would be nice to have some insight into any causes of performance regressions.

@egibs egibs requested a review from tstromberg May 10, 2024 01:24
@egibs egibs changed the title Add profiling to help with OOM/performance Add profiling to help with OOM/performance investigations May 10, 2024
Copy link
Collaborator

@tstromberg tstromberg left a comment

Choose a reason for hiding this comment

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

This is great! I'm going to use this today.

bincapz.go Outdated
@@ -9,13 +9,15 @@ import (
"flag"
"fmt"
"io/fs"
"log"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than adding a second logging library, please re-use the clog import - even if, admittedly, I prefer the built-in Go library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in c978170 (#207).

return stop, nil
}

func HandleProfileFlag(pf *bool) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move all flag handling to main.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in c978170 (#207).

log.Fatal("render failed", slog.Any("error", err))
}

if *profileFlag {
Copy link
Member Author

Choose a reason for hiding this comment

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

This avoids os.Exit(1) and defer not playing nicely together.

@egibs egibs requested a review from tstromberg May 10, 2024 13:08
Copy link
Collaborator

@tstromberg tstromberg left a comment

Choose a reason for hiding this comment

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

nice work!

@tstromberg tstromberg merged commit 6049524 into chainguard-dev:main May 10, 2024
7 of 8 checks passed
@egibs egibs deleted the add-profiling branch June 10, 2024 14:22
egibs pushed a commit to egibs/malcontent that referenced this pull request Aug 5, 2024
…-dev#207)

* Add profiling to help with OOM/performance

* Profile doesn't need to be exported

* Address PR comments
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