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

runtime/pprof: better generic type information in pprof stack traces #68196

Open
felixge opened this issue Jun 26, 2024 · 6 comments
Open

runtime/pprof: better generic type information in pprof stack traces #68196

felixge opened this issue Jun 26, 2024 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest gabywins NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@felixge
Copy link
Contributor

felixge commented Jun 26, 2024

Proposal Details

tl;dr: Can we show something more descriptive than go.shape.*uint8 for generic methods invoked on a struct pointer Maybe go.shape.uintptr or the element type?


While profiling is primarily seen as a performance tool, we often see it used for understanding the control flow of Go applications at runtime. In particular, profiling can be very useful to reveal which concrete types are being invoked behind an interface or a generic function call.

In one particular case @g-talbot recently tried to figure out the concrete type parameter for a generic function call, and to our surprise we saw the type being called go.shape.*uint8 which was confusing since it doesn't correspond to any of the types satisfying the type constraints of the generic functions. We would have expected to see something more like *SomeStruct instead.

After a bit of digging, my colleague @nsrip-dd discovered that this output is probably intentional (source), but we're wondering if it could be made more user-friendly. Either by clarifying that the type is an unknown pointer type (go.shape.uintptr) or by showing the actual type.

Below is a minimal example that shows how to end up with a go.shape.*uint8 inside of a CPU profile:

package main

import "testing"

func BenchmarkContainer(b *testing.B) {
	d := &Container[*Node]{}
	for i := 0; i < b.N; i++ {
		d.Add(nil)
	}
}

type Container[T any] struct{}

//go:noinline to make this frame shows up in the cpu profile.
func (d *Container[T]) Add(a T) {}

type Node struct{}
$ go test -bench . -cpuprofile cpu.pprof
$ go tool pprof -http=: cpu.pprof.

image

We understand that the current displayed type probably makes a lot of sense from the compiler's perspective, but seeing a uint8 type as a placeholder for a pointer type on a 64 bit system was confusing to us.

@gopherbot gopherbot added this to the Proposal milestone Jun 26, 2024
@gabyhelp
Copy link

Similar Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@felixge
Copy link
Contributor Author

felixge commented Jun 26, 2024

Good bot :). Based on the discussion in this issue, I understand that it might not be possible to do what's being suggested here. If that's the case I'd still suggest to leave the issue open for a bit to see how many Go users care about this problem.

@ianlancetaylor
Copy link
Contributor

Taking this out of the proposal process.

@ianlancetaylor ianlancetaylor changed the title proposal: better generic type information in pprof stack traces runtime/pprof: better generic type information in pprof stack traces Jun 26, 2024
@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Proposal labels Jun 26, 2024
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog Jun 26, 2024
@ianlancetaylor ianlancetaylor added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 26, 2024
@g-talbot
Copy link

In particular, what I'd like to add is that if the definition of the generic type in question is constrained on an interface, the debug information could at least contain that interface name. (In my particular case that I reported to @felixge this was true.)

Also I'm curious when there are different implementations of the interface, and the structure is like this:

type Container[T InterfaceType] struct{
    Value T
}

//go:noinline to make this frame shows up in the cpu profile.
func (d *Container[T]) Add(a T) {}

type InterfaceType interface {
  SomeMethod(int a) error
 }

Will the compiler ever generate different implementations of the various methods on Container[T] specifically for different T so that it could say which implementation it was using in the debug info? (i.e. like Container[BlueThing].Add() instead of Container[InterfaceType].Add()?)

@prattmic
Copy link
Member

Just brainstorming, but I think theoretically traceback could, upon encountering a type-parameterized function, look for the dictionary argument (assuming it is still live) and extracting the real type parameters. This would make traceback more expensive, but I think the dictionary does always contain the real types.

I think we'd also want to use both the name and system_name pprof fields to record both names: https://github.com/google/pprof/blob/main/proto/profile.proto#L218-L222

@prattmic
Copy link
Member

cc @golang/runtime

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. FeatureRequest gabywins 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

8 participants