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/compile: pgo support for external Linux perf profiles and perf_data_converter #64489

Open
zamazan4ik opened this issue Dec 1, 2023 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@zamazan4ik
Copy link

zamazan4ik commented Dec 1, 2023

Go PGO docs mentions some support for the external profile tooling and has a reference to the external page about supported PGO tools. Having support for the external profiling tooling is important since some organizations already have their profiling infrastructure (GWP in Google, Perforator in Yandex, sometimes even Grafana Pyroscope or similar in-house solutions).

The official documentation explicitly mentions widely used Linux' perf profiles for PGO purposes. Using these profiles are common thing in the C++ world due to sampling PGO support with AutoFDO tooling.

pprof package has a reference to a converter from Linux perf profiles to pprof profiles - https://github.com/google/perf_data_converter . Did anyone try to test it with Go PGO? Is it possible with this tool to use perf profiles to enable PGO optimization with the current PGO infrastructure in the Go compiler?

If it works, I suggest adding a mention about perf_data_converter to the https://github.com/golang/go/wiki/PGO-Tools page. Having a possibility to use Linux' perf profiles can be important since it allows users to implement company-wide PGO optimization pipelines in the same way as it's done for C++ infrastructure (infrastructure and tooling reusage, you know).

@seankhliao seankhliao changed the title External PGO profiles support: Linux perf profiles and perf_data_converter cmd/compile: pgo support for external Linux perf profiles and perf_data_converter Dec 1, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 1, 2023
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 1, 2023
@dmitshur dmitshur added this to the Backlog milestone Dec 1, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Dec 1, 2023

CC @golang/compiler.

@prattmic
Copy link
Member

prattmic commented Dec 6, 2023

https://github.com/google/perf_data_converter should be the way to do this, but there a couple of minor roadblocks:

  1. perf_data_converter doesn't symbolize its output profile (it just contains PCs). That's not a fundamental problem, as pprof will symbolize profiles, so you could do something like pprof -proto converted.pprof > symbolized.pprof. But this leads to problem 2.
  2. pprof symbolization doesn't set the Function.start_line field in the profiles, which Go's PGO requires. This should be a fairly small change to pprof to support, but it hasn't been done yet.

@zamazan4ik
Copy link
Author

zamazan4ik commented Dec 7, 2023

pprof symbolization doesn't set the Function.start_line field in the profiles, which Go's PGO requires. This should be a fairly small change to pprof to support, but it hasn't been done yet.

Do we need to create a separate issue in the pprof repo to add this functionality?

@prattmic
Copy link
Member

prattmic commented Dec 7, 2023

I filed google/pprof#823 for this, including a prototype implementation.

This made me realize one more tiny snag: the compiler is picky about the sample types reported by the profile (mostly to warn users if they try to use a heap profile or something), but the sample types selected by perf_to_profile aren't quite right. It is easy to change them [1], but we should probably just make the compiler less picky.

[1]

change_sample.go:

package main

import (
        "os"

        "github.com/google/pprof/profile"
)

func main() {
        p, _ := profile.Parse(os.Stdin)
        p.SampleType = []*profile.ValueType{
                {
                        Type: "event",
                        Unit: "count",
                },
                {
                        Type: "samples",
                        Unit: "count",
                },
        }
        p.Write(os.Stdout)
}
$ go run change_sample.go < perf.pprof > perf.changed.pprof

@insilications
Copy link

It would be nice to see perf_data_converter (which is used by pprof) being able to provide symbolize and the correct Function.start_line for golang's PGO.

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. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

5 participants