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

pprof provider #133

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

pprof provider #133

wants to merge 29 commits into from

Conversation

bevzzz
Copy link
Contributor

@bevzzz bevzzz commented Jun 6, 2024

Summary

pprof provider annotates functions/methods with CPU time / memory allocations associated with them.

Motivation

The idea came to me as I was reading about profiling Go programs with pprof. The provider consists of a simple Go parser and a minimal wrapper around go tool pprof / pprof. When called, it will search through the user's workspace to find a file matching the "pprof report" glob and collect statics for each function declared in this file.

Below is the example of the annotations created for the loop finding algorithm from the Profiling Go Programs blog post.

Annotations Function
image Screenshot 2024-06-06 at 22 58 29

The way I imagine it, such annotations would allow Cody to assist developers at tackling performance bottlenecks by making suggestions based on factual data (metrics). I still haven't figured out a way to make this context available to Cody -- that's something for tomorrow's pairing session.

TODOs:

  • Include data from memory profile reports
  • Support standalone pprof (not as part of go binary)
  • Find binary from which the report was generated. Passing it as an argument to pprof unlocks some additional functionality and makes the output more precise.
  • Make this info available to Cody
    Upd: Not something we can do just now, needs a separate implementation in Cody.
  • Docs

Some additional features that I have in mind for it:

  • Annotating "resource hungry" lines using the detailed output of pprof -list
    Upd: for now just passing the output of this command to annotations.items.ai
  • Adding a preview of the function's call graph with pprof svg
  • Support other languages: there are profiling tools which generate pprof-compatible reports for other languages (e.g. pprof-nodejs)

Try it out

To use this provider you'll need to:

  • Check out the branch
  • Find a Go project and run a profiler on it
  • Enable the openctx/provider/pprof provider and give it the report glob to look for
  • Open a Go file (mind that not every file will have functions with CPU times significant enough to be included in pprof's output)

Copy link
Member

@keegancsmith keegancsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. we have discussed a way to send over in mentions/items some editor context. But thinking a bit more about it, maybe we should add a way to mention the current files annotations (or selected lines annotations)? The annotation response has an item field which can then be used for context. cc @sqs @thenamankumar

@bevzzz
Copy link
Contributor Author

bevzzz commented Jun 7, 2024

But thinking a bit more about it, maybe we should add a way to mention the current files annotations (or selected lines annotations)?

I'm thinking about looking at the Cody repo after I'm done with this PR, maybe that's something I could work on?

@bevzzz bevzzz force-pushed the feat/provider-pprof branch from d25fd15 to a9b7632 Compare June 7, 2024 08:57
@bevzzz bevzzz force-pushed the feat/provider-pprof branch 4 times, most recently from 6a63169 to fd348b8 Compare June 13, 2024 00:09
@bevzzz bevzzz marked this pull request as ready for review June 13, 2024 00:13
Before generating annotations for a file, we want to check if
1) go is installed (to use `go tool pprof`)
2) a pprof report that matches the configured glob exists in
one of the parent directories
This makes sure the annotations are only set for the most "resource
hungry" functions and not for each function in the package.
Putting it all together!

`profider-pprof` can now annotate Go source files with information about
the CPU time the functions/methods defined in it take.
This requires `go` to be installed (standalone `pprof` will be supported
too) and a report generated.

Some remarks:
- Bungling to --format=cjs until this fix
sourcegraph#131 is merged
- _log.ts file is a temporary fixture for local development
- Lots of TODOs to handle
execSync does not return an error if the command
fails, so we check that its output is not "not found".
Usually pprof is included in go distribution, but the extention should
be able to use either of the tools, whichever one's installed.
This should allow users to specify directories where the pprof reports
should and shouldn't be taken from.
In the `annotations()` we would previously loop over every function in a file, then loop over the function from `pprof top` to find information for the given function.
Since "top" results should remain ordered, it seems easier to return funcs as a Record<string, Func>
Previously reportGlob, binaryGlob, and all TopOptions
were ignored even if set in settings.json.
When searching for the Go binary, we assume that it will be in the same directory as the report, or in one of its child directories.
If we break the loop the moment we find the report, then we might not find the binary even if it's in the same directory but is alphabetically sorted after the report file.
@bevzzz bevzzz force-pushed the feat/provider-pprof branch from 15943dc to 28f4f54 Compare December 12, 2024 13:34
bevzzz added a commit to bevzzz/provider-pprof that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants