fix(pgo): skip re-symbolization when merging pprof profiles#2407
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the PGO profile merge step to avoid pprof re-symbolization that can drop Function.start_line and cause Go’s preprofile to reject the merged profile when building frankenphp-pgo.
Changes:
- Add
-symbolize=noneto thepprof -protomerge invocation to preserve existing function metadata. - Document why re-symbolization is avoided, referencing the upstream
google/pprofissue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pprof -proto re-symbolized the merged profile and produced Function records without start_line, which Go PGO's preprofile rejects with "profile missing Function.start_line data" (google/pprof#823). Pass -symbolize=none so the already-symbolized input from net/http/pprof is preserved.
dunglas
force-pushed
the
fix/pgo-merge-symbolize-none
branch
from
ccf7fe5 to
bd537b3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The PGO refresh workflow at https://github.com/php/frankenphp/actions/runs/25779871543/job/75719971682 (and the two prior runs - the job has never succeeded) failed when building
frankenphp-pgowith:The two input profiles produced by
net/http/pprofalready carryFunction.start_line(Go 1.20+ guarantees this). The problem is the merge step itself:pprof -protoruns local symbolization by default. On the CI runner that falls back toaddr2line, whose adapter in https://github.com/google/pprof/blob/main/internal/binutils/addr2liner.go constructsplugin.Framewith onlyFunc,File, andLine.StartLineis never set, so the newFunctionrecords that replace the original symbolized ones come out withstart_line = 0, which Go PGO'spreprofilerejects.pprof -symbolize=noneskips that step so the already-symbolized records fromnet/http/pprofsurvive the merge intact. (The PGO Tools wiki notes that pprof "should produce a PGO-compatible output" only when its symbolization preserves metadata - that's true with llvm-symbolizer's JSON path added in google/pprof#891, but not with the addr2line fallback used in CI.)