Navigation Menu

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

Jump to bottom

runtime/pprof,cmd/compile: simplified symbol names for generics break PGO matching #58712

Closed
prattmic opened this issue Feb 24, 2023 · 4 comments
Closed

runtime/pprof,cmd/compile: simplified symbol names for generics break PGO matching #58712

prattmic opened this issue Feb 24, 2023 · 4 comments
Assignees
@cherrymui
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.21

Comments

@prattmic
Copy link
Member

prattmic commented Feb 24, 2023 •
edited

For functab, and thus stack traces and symbol names in pprof profile outputs, the compiler simplifies symbol names like foo[go.shape.int64] to just foo[...].

When reading a profile for PGO, the compiler will attempt to match symbols and fail because the profile symbol name does not match the actual symbol name.

This could be addressed from either end:

  • The profile could contain the full symbol name (Function.name = "foo[...]", Function.system_name = "foo[go.shape.int64]" perhaps?).
  • Or, the compiler could match foo[...] to any instantiation of foo.

cc @cherrymui @aclements
@prattmic prattmic added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 24, 2023
@prattmic prattmic added this to the Go1.21 milestone Feb 24, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 24, 2023
@cherrymui
Copy link
Member

I'd prefer the first option. For the same generic function, some instantiation may be hot and some other may be cold. So it would be good to distinguish them. Especially if we want to do PGO-based specification/stenciling later.

@cherrymui
Copy link
Member

I'll look into it.

@cherrymui cherrymui self-assigned this Feb 24, 2023
@gopherbot
Copy link

Change https://go.dev/cl/491677 mentions this issue: cmd/link, runtime: include full symbol name for generic functions in runtime table

@gopherbot
Copy link

Change https://go.dev/cl/491678 mentions this issue: runtime, runtime/pprof: record instantiated symbol name in CPU profile

gopherbot pushed a commit that referenced this issue May 5, 2023
@cherrymui
cmd/link, runtime: include full symbol name for generic functions in …
1a61278 
…runtime table

For generic functions and methods, we replace the instantiated
shape type parameter name to "...", to make the function name
printed in stack traces looks more user friendly. Currently, this
is done in the binary's runtime func table at link time, and the
runtime has no way to access the full symbol name. This causes
the profile to also contain the replaced name. For PGO, this also
cause the compiler to not be able to find out the original fully
instantiated function name from the profile.

With this CL, we change the linker to record the full name, and
do the name substitution at run time when a printing a function's
name in traceback.

For #58712.

Change-Id: Ia0ea0989a1ec231f3c4fbf59365c9333405396c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/491677
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cherrymui cherrymui

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
3 participants
@prattmic @gopherbot @cherrymui