Skip to content

cmd: introduce profiling options#516

Merged
lzap merged 2 commits into
osbuild:mainfrom
lzap:pprof-opts1
May 21, 2026
Merged

cmd: introduce profiling options#516
lzap merged 2 commits into
osbuild:mainfrom
lzap:pprof-opts1

Conversation

@lzap
Copy link
Copy Markdown
Contributor

@lzap lzap commented May 13, 2026

Add persistent flags --memprofile, --memprofile-goroutine, and --memprofile-rate. Heap and goroutine profiles are written in Go pprof format when the process exits, including on command failure, so runs can be analyzed with the toolchain from go.mod (e.g. go tool pprof).

When profiling is off, allocation sampling is not enabled for the image-builder CLI: MemProfileRate is reset at the start of run() and stays zero unless a command sets --memprofile, in which case MemProfileRate follows --memprofile-rate (-1 means Go's default 524288).

image-builder -v manifest ... --memprofile /tmp/heap.pprof \
    --memprofile-goroutine /tmp/goroutine.pprof
go tool pprof -http=:8080 /tmp/heap.pprof

@croissanne
Copy link
Copy Markdown
Member

croissanne commented May 13, 2026

Would it make sense to use build tags here? That way we can add them for debug builds and stuff. I'm not sure if we want to have these flags be part of our release builds.

Copy link
Copy Markdown
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

I'd like to have these things but like @croissanne says they should be behind a build flag/tag so we don't ship this in production executables.

@lzap
Copy link
Copy Markdown
Contributor Author

lzap commented May 14, 2026

Sure, rebased.

Comment thread cmd/image-builder/memprofile.go
brlane-rht
brlane-rht previously approved these changes May 14, 2026
Copy link
Copy Markdown

@brlane-rht brlane-rht left a comment

Choose a reason for hiding this comment

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

Looks good other than my one small comment.

@lzap
Copy link
Copy Markdown
Contributor Author

lzap commented May 14, 2026

Rebased your suggestion, also added a bit of documentation to each method.

Copy link
Copy Markdown

@brlane-rht brlane-rht left a comment

Choose a reason for hiding this comment

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

Looks good.

@supakeen supakeen added this pull request to the merge queue May 20, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 20, 2026
@supakeen
Copy link
Copy Markdown
Member

supakeen commented May 20, 2026

Will need a rebase on #520 which contains the fix for CI. Let's do that after it hits main so we don't lose acks here.

lzap and others added 2 commits May 21, 2026 13:35
After json.Indent fills a bytes.Buffer, stream the buffer to the output writer with WriteTo instead of fmt.Fprintf(..., pretty.String()). That avoids allocating a second copy of the entire indented manifest as a string, which matters for large manifests.

Co-authored-by: Cursor <cursoragent@cursor.com>
These are only available when building with the DEBUG=1 make target.
@lzap
Copy link
Copy Markdown
Contributor Author

lzap commented May 21, 2026

Rebased.

@lzap lzap enabled auto-merge May 21, 2026 11:36
@lzap lzap added this pull request to the merge queue May 21, 2026
Merged via the queue into osbuild:main with commit 0e02b05 May 21, 2026
39 checks passed
@lzap lzap deleted the pprof-opts1 branch May 21, 2026 12:21
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.

4 participants