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

cmd/pgopack: cache packed pgo profile during build #62400

Closed
jinlin-bayarea opened this issue Aug 31, 2023 · 10 comments
Closed

cmd/pgopack: cache packed pgo profile during build #62400

jinlin-bayarea opened this issue Aug 31, 2023 · 10 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. ToolSpeed
Milestone

Comments

@jinlin-bayarea
Copy link
Contributor

The PGO-enabled Go compilation shows the significant latency improvement of generated binary which benefits from profile-guided inlining and code specialization. However, the workflow of processing and extracting profiling data is surprisingly suboptimal in the community Go compiler. Based on the observation and instrument timing data from small benchmark and real service, we notice that the overall pprof data parsing time varies from 70% to over 95% of total time when PGO flow is enabled in the compilation. Furthermore, the existing PGO flow reads and parses the pprof file in every single package compilation and the cumulative time of this process is long and unnecessary. One-shot processing profiling data and cache information in a well-designed format for optimization transformation among all packages is critical.

To optimize the existing flow and reduce build time of multiple Go services and projects, we propose to create a standalone tool, PGO preprocessor, to extract information from collected profiling files and to cache the WeightedCallGraph in one time fashion. By adding the new tool to the Go compiler, it reduces the time of repeated profiling file parsing in current Go Compiler. The new tool is capable of all existing PGO enabled optimizations including the inlining and devirtualization in the Go compiler.

In summary, we propose adding a standalone preprocess tool to reduce the compile time when the PGO is enabled. Inputs are welcome.

@gopherbot gopherbot added this to the Proposal milestone Aug 31, 2023
@cespare
Copy link
Contributor

cespare commented Aug 31, 2023

For the record, speeding up the underlying work is #58102.

@cespare cespare added compiler/runtime Issues related to the Go compiler and/or runtime. ToolSpeed labels Aug 31, 2023
@cespare
Copy link
Contributor

cespare commented Aug 31, 2023

@jinlin-bayarea Are your .pgo files in version control, so that all builds use PGO? I know that's the Go team's recommendation (here, for example).

At my company we've only started playing around with PGO a bit but our current approach is to only build with PGO as part of production deploys, not local development/testing. So having PGO builds be a bit slower doesn't really matter much.

(We don't want to put the .pgo files in our repo because we are collecting very large profiles and we are updating them with automated processes.)

@jinlin-bayarea
Copy link
Contributor Author

The pgo files are not inversion control. We have collected our pgo profiles daily and always use the most recent profile to build the service code.

@prattmic
Copy link
Member

cc @aclements @cherrymui

@cherrymui
Copy link
Member

cherrymui commented Sep 19, 2023

Discussed briefly offline. The idea of preprocessing sounds good overall.

  • In the usual case we'd want the go command to automatically run the preprocessor and cache the result. So this step is transparent to the user, and go build -pgo still works as before.
  • The preprocessor can be a standalone command, so external build tools like Bazel can invoke it (and do its own caching).

It would be good if you could share a prototype CL. Then we can discuss the details from there, like the exact format of the preprocessed profile. Thanks.

@jinlin-bayarea
Copy link
Contributor Author

Hi Cherry. I have submitted the prototype in this CL. https://go-review.googlesource.com/c/go/+/529738.
Please help review the changes. Thanks.

@rsc
Copy link
Contributor

rsc commented Nov 1, 2023

I would suggest calling it pgopack or something like that instead of "preprofile" since Go has lots of profiles. Otherwise this seems fine, and it should be invisible to users, so I think it can proceed with compiler/runtime team agreement instead of a proposal.

/cc @bcmills

@cherrymui cherrymui removed the Proposal label Nov 1, 2023
@cherrymui cherrymui changed the title proposal: cmd/preprofile: perform preprocessing on the profile file to accelerate the compilation process cmd/preprofile: perform preprocessing on the profile file to accelerate the compilation process Nov 1, 2023
@rsc rsc changed the title cmd/preprofile: perform preprocessing on the profile file to accelerate the compilation process cmd/pgopack: cache packed pgo profile during build Nov 1, 2023
@cherrymui
Copy link
Member

Taking out of proposal as this is invisible to users using the go command. The tool will be internal (the go command will run it automatically if needed), so is the preprocessed profile format. Users are not expected to check in profiles generated by the preprocessing tool.

@bcmills bcmills added the GoCommand cmd/go label Nov 1, 2023
@jinlin-bayarea
Copy link
Contributor Author

Agree with @cherrymui . For bazel build users, they need to use the preprocess command separately to retrieve the preprocessed profile.

@cherrymui cherrymui modified the milestones: Proposal, Backlog Nov 2, 2023
@cherrymui cherrymui added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 2, 2023
@prattmic
Copy link
Member

Tool is submitted in https://go.dev/cl/529738. It still needs integration with cmd/go. I'm going to close this issue in favor of #58102, which is effectively a duplicate of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. ToolSpeed
Projects
None yet
Development

No branches or pull requests

7 participants