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

cmd/compile: very long names for simple generic usage #71535

Open
rsc opened this issue Feb 2, 2025 · 4 comments
Open

cmd/compile: very long names for simple generic usage #71535

rsc opened this issue Feb 2, 2025 · 4 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Feb 2, 2025

Go version

go1.23.5, go1.24rc2

Output of go env in your module/workspace:

n/a

What did you do?

Build a program using a few TransientSlice[Value].
Profile it, showing hot symbols.
Run nm to see symbols.

git clone https://github.com/rsc/ivy
cd ivy
git fetch pprofbug # tag I created
go build
go tool nm ./ivy | grep Append
./ivy -profile=x.prof testdata/copy.ivy
go tool pprof ./ivy
top10

What did you see happen?

Unexpectedly long symbol names, like in the comically large and unreadable boxes in this profile:

Image

Or in this nm output:

% go tool nm ivy | grep Append
1000cbb10 T math/big.(*Float).Append
100131f60 T robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.interface { Eval(robpike.io/ivy/value.Context) robpike.io/ivy/value.Value; Inner() robpike.io/ivy/value.Value; ProgString() string; Rank() int; Sprint(*robpike.io/ivy/config.Config) string; String() string; robpike.io/ivy/value.shrink() robpike.io/ivy/value.Value; robpike.io/ivy/value.toType(string, *robpike.io/ivy/config.Config, robpike.io/ivy/value.valueType) robpike.io/ivy/value.Value }]).Append
1000837d0 T strconv.AppendInt
1000944e0 T time.Time.AppendFormat
% 

What did you expect to see?

Reasonably short symbol names, and normal-sized pprof boxes.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 2, 2025
@rsc
Copy link
Contributor Author

rsc commented Feb 2, 2025

Note that the actual code uses an interface, not that long literal.

% grep TransientSlice value/*.go
value/vector.go:	t *persist.TransientSlice[Value]
value/vector.go:	t := new(persist.TransientSlice[Value])
% 

Since the Value interface mentions Value itself, basically no one is ever going to write that literal accidentally, so there is arguably no reason to expand Value into the long literal. In the case where the argument is a named interface that mentions itself, or a named interface with more than N methods, it seems like it would be better to just use the interface name, so that these would show up in the code as

robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.robpike.io/ivy/value.Value]).Append

That's still longer than one would hope, but far shorter than the current symbol names.

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Feb 2, 2025
@rsc
Copy link
Contributor Author

rsc commented Feb 3, 2025

Another example of the long names showing up in an unwanted place: race detector reports.

WARNING: DATA RACE
Read at 0x00c000788268 by goroutine 4853:
  robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.interface { Eval(robpike.io/ivy/value.Context) robpike.io/ivy/value.Value; Inner() robpike.io/ivy/value.Value; ProgString() string; Rank() int; Sprint(*robpike.io/ivy/config.Config) string; String() string; robpike.io/ivy/value.shrink() robpike.io/ivy/value.Value; robpike.io/ivy/value.toType(string, *robpike.io/ivy/config.Config, robpike.io/ivy/value.valueType) robpike.io/ivy/value.Value }]).wnode()
      /Users/rsc/src/robpike.io/ivy/internal/persist/slice.go:251 +0x90
  robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.interface { Eval(robpike.io/ivy/value.Context) robpike.io/ivy/value.Value; Inner() robpike.io/ivy/value.Value; ProgString() string; Rank() int; Sprint(*robpike.io/ivy/config.Config) string; String() string; robpike.io/ivy/value.shrink() robpike.io/ivy/value.Value; robpike.io/ivy/value.toType(string, *robpike.io/ivy/config.Config, robpike.io/ivy/value.valueType) robpike.io/ivy/value.Value }]).Set()
      /Users/rsc/src/robpike.io/ivy/internal/persist/slice.go:322 +0x2d4
  robpike.io/ivy/value.(*vectorEditor).Set()
      /Users/rsc/src/robpike.io/ivy/value/vector.go:74 +0x368
  robpike.io/ivy/value.IndexAssign.func2()
      /Users/rsc/src/robpike.io/ivy/value/index.go:353 +0x2e4
  robpike.io/ivy/value.pfor.func1()
      /Users/rsc/src/robpike.io/ivy/value/eval.go:223 +0x90

Previous write at 0x00c000788268 by goroutine 4852:
  robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.interface { Eval(robpike.io/ivy/value.Context) robpike.io/ivy/value.Value; Inner() robpike.io/ivy/value.Value; ProgString() string; Rank() int; Sprint(*robpike.io/ivy/config.Config) string; String() string; robpike.io/ivy/value.shrink() robpike.io/ivy/value.Value; robpike.io/ivy/value.toType(string, *robpike.io/ivy/config.Config, robpike.io/ivy/value.valueType) robpike.io/ivy/value.Value }]).init()
      /Users/rsc/src/robpike.io/ivy/internal/persist/slice.go:213 +0x138
  robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.interface { Eval(robpike.io/ivy/value.Context) robpike.io/ivy/value.Value; Inner() robpike.io/ivy/value.Value; ProgString() string; Rank() int; Sprint(*robpike.io/ivy/config.Config) string; String() string; robpike.io/ivy/value.shrink() robpike.io/ivy/value.Value; robpike.io/ivy/value.toType(string, *robpike.io/ivy/config.Config, robpike.io/ivy/value.valueType) robpike.io/ivy/value.Value }]).wnode()
      /Users/rsc/src/robpike.io/ivy/internal/persist/slice.go:252 +0xf0
  robpike.io/ivy/internal/persist.(*TransientSlice[go.shape.interface { Eval(robpike.io/ivy/value.Context) robpike.io/ivy/value.Value; Inner() robpike.io/ivy/value.Value; ProgString() string; Rank() int; Sprint(*robpike.io/ivy/config.Config) string; String() string; robpike.io/ivy/value.shrink() robpike.io/ivy/value.Value; robpike.io/ivy/value.toType(string, *robpike.io/ivy/config.Config, robpike.io/ivy/value.valueType) robpike.io/ivy/value.Value }]).Set()
      /Users/rsc/src/robpike.io/ivy/internal/persist/slice.go:322 +0x2d4
  robpike.io/ivy/value.(*vectorEditor).Set()
      /Users/rsc/src/robpike.io/ivy/value/vector.go:74 +0x368
  robpike.io/ivy/value.IndexAssign.func2()
      /Users/rsc/src/robpike.io/ivy/value/index.go:353 +0x2e4
  robpike.io/ivy/value.pfor.func1()
      /Users/rsc/src/robpike.io/ivy/value/eval.go:223 +0x90
...

@mknyszek mknyszek added this to the Backlog milestone Feb 5, 2025
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 5, 2025
@mknyszek
Copy link
Contributor

mknyszek commented Feb 5, 2025

Related to #60324.

@seankhliao
Copy link
Member

also related? #50438

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. 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

5 participants