search

felixge / fgprof

🚀 fgprof is a sampling Go profiler that allows you to analyze On-CPU as well as Off-CPU (e.g. I/O) time together.
MIT License
2.81k stars 88 forks source link

Automatically include fgprof in /debug/index #5

Closed brancz closed 1 year ago

brancz commented 3 years ago

There is an open issue on conprof (a continuous profiling project), discussing how there could be support for fgprof. It more generally started a discussion around custom/non-built-in profiles.

I haven't had a chance to dig into the fgprof code a lot yet, but looking at the bits and pieces involved, it appears that if profiles are created using the pprof.NewProfile function, then they are automatically included in profiles returned by pprof.Profiles(), which is what the /debug/pprof index page calls to retrieve the list of available profiles.

I'll try to understand the code more and see if this is possible, but maybe you can tell directly off the top of your head, and whether if possible this is something you would accept a patch for? :)

@felixge

felixge commented 3 years ago

@brancz my understanding is that the current profile extension mechanism isn't suitable for fgprof.

The problem is that the only way to add a stack trace to a *Profile is func (p *Profile) Add(value interface{}, skip int) which doesn't allow to pass a stack as a parameter. Instead the stack trace is captured inside of the Add() func based on the current caller stack. So there is no way to add the stacks of other goroutines to a *Profile, only the stack of the profiler goroutine could be added.

That being said, if you can think of a good way to inject the stack of other goroutines into *Profile, I'd be open to supporting this mechanism : ).

brancz commented 3 years ago

I was afraid that is the case by reading over the code, but was hoping I was wrong :)

I'm gonna try and see if go might be interested in having a mechanism for this for profiles like fgprof. It doesn't seem totally wild considering that there is already the mechanism with pprof.NewProfile().

felixge commented 3 years ago

@brancz yeah, something like func (p *Profile) AddStack(value interface{}, stack runtime.StackRecord) might do the trick. But I'd have to prototype it a bit more to see if it would work with how the rest of the *Profiler extension mechanism works.

Anyway, feel free to suggest it to the Go folks, it would be a nice improvement.

felixge commented 1 year ago

Closing this since the upstream proposal was rejected.