Open
Conversation
keegancsmith
reviewed
Jun 7, 2024
Member
keegancsmith
left a comment
keegancsmith
left a comment
There was a problem hiding this comment.
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
keegancsmith
reviewed
Jun 7, 2024
Contributor
Author
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
force-pushed
the
feat/provider-pprof
branch
from
d25fd15 to
a9b7632
Compare
RXminuS
reviewed
Jun 7, 2024
bevzzz
force-pushed
the
feat/provider-pprof
branch
4 times, most recently
from
6a63169 to
fd348b8
Compare
tinvaan
reviewed
Jun 13, 2024
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
force-pushed
the
feat/provider-pprof
branch
from
15943dc to
28f4f54
Compare
bevzzz
added a commit
to bevzzz/provider-pprof
that referenced
this pull request
Dec 12, 2024
as a standalone npm module.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pprofprovider 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 aroundgo 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.
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:
pprof(not as part ofgobinary)pprofunlocks some additional functionality and makes the output more precise.Make this info available to CodyUpd: Not something we can do just now, needs a separate implementation in Cody.
Some additional features that I have in mind for it:
pprof -listUpd: for now just passing the output of this command to
annotations.items.aipprof svgpprof-compatible reports for other languages (e.g.pprof-nodejs)Try it out
To use this provider you'll need to:
openctx/provider/pprofprovider and give it the report glob to look forpprof's output)