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

go method naming convention used by capslock is inconsistent with other popular tools #115

Open
thanm opened this issue May 6, 2024 · 1 comment

Comments

@thanm
Copy link

thanm commented May 6, 2024

The naming convention that capslock uses for Go methods seems to be inconsistent with the conventions used by other tools (for example, debuggers, profilers, and so on).

Here is a toy program to demonstrate: playground link.

This program makes a series of calls into the text/template program from Go's standard library, at runtime most of the time is spent in that package. If I run this program through capslock, the convention used for reporting pointer Go methods is

(*<packagepath>.<type>).<methodname>

instead of the more commonly used

<packagepath>.(*<type>).<methodname>

Here is what I see from capslock:

$ capslock -output=graph > p.dot
$ fgrep '.walk"' *.dot | head -3
	"(*text/template.Template).execute" -> "(*text/template.state).walk"
	"(*text/template.state).walk" -> "(*text/template.state).errorf"
	"(*text/template.state).walk" -> "(*text/template.state).evalPipeline"
$

Examples of other tools that use the latter convention: pprof (profiler), delve (debugger). Specifics:

Profiler:

$ go run hotmeth.go 
$ ls *.p
prof.p
$ pprof prof.p
File: hotmeth
Type: cpu
Time: May 6, 2024 at 1:50pm (UTC)
Duration: 1.41s, Total samples = 1.40s (99.57%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top10
Showing nodes accounting for 520ms, 37.14% of 1400ms total
Showing top 10 nodes out of 210
      flat  flat%   sum%        cum   cum%
     120ms  8.57%  8.57%      340ms 24.29%  runtime.mallocgc
      80ms  5.71% 14.29%      280ms 20.00%  text/template/parse.(*lexer).nextItem
      50ms  3.57% 17.86%      560ms 40.00%  text/template/parse.(*Tree).textOrAction
      40ms  2.86% 20.71%       60ms  4.29%  reflect.(*structType).FieldByName
      40ms  2.86% 23.57%      120ms  8.57%  text/template.(*state).evalField
      40ms  2.86% 26.43%      300ms 21.43%  text/template.(*state).walk
      40ms  2.86% 29.29%      250ms 17.86%  text/template/parse.(*Tree).nextNonSpace
      40ms  2.86% 32.14%      740ms 52.86%  text/template/parse.(*Tree).parse
      40ms  2.86% 35.00%      130ms  9.29%  text/template/parse.(*Tree).peek
      30ms  2.14% 37.14%       50ms  3.57%  runtime.deductAssistCredit
(pprof)  

Debugger:

$ go build hotmeth.go
$ dlv debug .
Type 'help' for list of commands.
(dlv) b text/template.(*state).walk
Breakpoint 1 set at 0x586336 for text/template.(*state).walk() /w/ygo/src/text/template/exec.go:261
(dlv) c
> text/template.(*state).walk() /w/ygo/src/text/template/exec.go:261 (hits goroutine(1):1 total:1) (PC: 0x586336)
   256:		walkContinue = errors.New("continue")
   257:	)
   258:	
   259:	// Walk functions step through the major pieces of the template structure,
   260:	// generating output as they go.
=> 261:	func (s *state) walk(dot reflect.Value, node parse.Node) {
   262:		s.at(node)
   263:		switch node := node.(type) {
   264:		case *parse.ActionNode:
   265:			// Do not pop variables so they persist until next end.
   266:			// Also, if the action declares variables, don't print the result.
(dlv) b (*text/template.state).walk
Command failed: location "(*text/template.state).walk" not found
(dlv)

It would be nice if capslock could work the same way. Thanks.

@jcd2
Copy link
Collaborator

jcd2 commented May 21, 2024

So as background for the thread (I know you know this already :)) -- there's one set of logic in the compiler for producing symbols and DWARF information for the executable, and another set of logic in golang.org/x/tools and go/types. The names produced by the former are seen by tools like pprof, delve, and objdump that work with executables, and the latter is used by analysis tools like x/tools/cmd/{callgraph,ssadump} and by capslock. Capslock is calling the String method on the x/tools/go/ssa.Function object to get the name.

There are many cases where the generated names and even the set of actual functions are different between the two (synthetic functions, anonymous functions, nested anonymous functions, coalescing of instantiations, method expressions, defer, init, etc.) so getting these to match exactly is not simple, but we should be able to match the compiler for common cases.

Perhaps that should be done upstream in x/tools too, but that might be too disruptive, even though the strings are documented as being subject to change.

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

No branches or pull requests

2 participants