Conversation
The pprof file was a merge from pprof taken from a production block producer and a sentry. This profile allows the go compiler to make more informed optimization decisions during compilation. For more info, see: https://go.dev/doc/pgo
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward addition of the PGO flag to the build target.
Extended reasoning...
Overview
This PR adds Profile-Guided Optimization (PGO) to the make bor build target by including a -pgo=prod.pprof flag and committing a merged production pprof profile. The change touches two files: a one-line modification to the Makefile and the addition of the binary prod.pprof profile.
Security risks
No security concerns. The change only affects compiler optimization behavior via a standard Go feature (-pgo). The pprof file is a CPU profile used by the compiler — it contains no secrets or executable code.
Level of scrutiny
This is a low-risk, mechanical change. PGO is a well-documented Go compiler feature (available since Go 1.21) that uses profiling data to guide optimization decisions. The worst-case outcome is that the profile provides no benefit; it cannot introduce correctness issues. The inline bug comment about the goreleaser config not including the flag is a valid enhancement suggestion but does not affect the correctness of this change.
Other factors
The PR is small and self-contained with clear intent described in the PR description. The nit about .goreleaser.yml missing the PGO flag for official release builds is already raised as an inline comment and is worth the author considering, but it does not block this change.
| bor: | ||
| mkdir -p $(GOPATH)/bin/ | ||
| go build -o $(GOBIN)/bor $(GO_LDFLAGS) ./cmd/cli/main.go | ||
| go build -o $(GOBIN)/bor -pgo=prod.pprof $(GO_LDFLAGS) ./cmd/cli/main.go |
There was a problem hiding this comment.
🟡 The -pgo=prod.pprof flag is added to make bor but not to .goreleaser.yml, which builds the official release binaries (darwin-amd64, darwin-arm64, linux-amd64, linux-arm64). Official releases distributed via GitHub Releases, Docker images, and deb/rpm/apk packages will not benefit from PGO. Consider adding goflags: ["-pgo=prod.pprof"] to each build target in .goreleaser.yml.
Extended reasoning...
What the bug is
This PR adds -pgo=prod.pprof to the make bor target in the Makefile (line 29), enabling Profile-Guided Optimization for local builds. However, the .goreleaser.yml file, which defines how official release binaries are built, does not include this flag in any of its four build configurations.
The specific code paths affected
The .goreleaser.yml file defines four build targets: darwin-amd64 (lines 9-22), darwin-arm64 (lines 24-37), linux-amd64 (lines 39-53), and linux-arm64 (lines 55-69). Each target only specifies ldflags: -s -w (with some adding -extldflags "-static" for Linux). None of them include a goflags setting that would pass -pgo=prod.pprof to the Go compiler.
Step-by-step proof
- A maintainer tags a new release (e.g.,
v1.6.0). - The GitHub Actions release workflow (
.github/workflows/release.yml) triggers and runsmake release. - The
make releasetarget (Makefile lines ~203-215) launches a goreleaser Docker container. - Goreleaser reads
.goreleaser.ymland builds four binaries using onlyldflags: -s -w— no-pgoflag is passed. - The resulting binaries are published to GitHub Releases, packaged into Docker images, and distributed as deb/rpm/apk packages — all without PGO optimizations.
- Meanwhile, anyone building locally with
make borgets PGO-optimized binaries.
Impact
The majority of production deployments likely use official release artifacts rather than building from source. This means the primary intended beneficiaries of this PR — production nodes — will not actually receive PGO-optimized binaries. The prod.pprof file (merged from production block producer and sentry profiles) would go unused in the release pipeline.
How to fix
Add goflags: ["-pgo=prod.pprof"] to each of the four build targets in .goreleaser.yml. Note that the prod.pprof file is at the repo root and the goreleaser Docker container mounts the repo at /go/src/$(PACKAGE_NAME), so the relative path should work. Cross-compilation targets should still benefit from PGO since the profile is architecture-independent at the Go compiler level.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2138 +/- ##
===========================================
+ Coverage 51.50% 51.54% +0.04%
===========================================
Files 882 882
Lines 154081 154081
===========================================
+ Hits 79352 79420 +68
+ Misses 69552 69492 -60
+ Partials 5177 5169 -8 see 19 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
4 participants
Description
The pprof file was a merge from pprof taken from a production block producer and a sentry. This profile allows the go compiler to make more informed optimization decisions during compilation. For more info, see: https://go.dev/doc/pgo