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: inlined function with interfaces should promote to concrete types #25791

Closed
dsnet opened this issue Jun 7, 2018 · 2 comments
Closed

Comments

@dsnet
Copy link
Member

@dsnet dsnet commented Jun 7, 2018

Consider the following code:

func main() {
	new(FooMessage).ProtoReflect().Type()
}

type FooMessage fooMessage

func (m *FooMessage) ProtoReflect() Message {
	return (*fooMessage)(m)
}

type fooMessage struct{}

func (m *fooMessage) Type() string {
	return "test"
}

type Message interface {
	Type() string
}

When compiled with -gcflags=-m, you see:

./main.go:11:6: can inline (*FooMessage).ProtoReflect
./main.go:17:6: can inline (*fooMessage).Type
./main.go:6:30: inlining call to (*FooMessage).ProtoReflect
./main.go:6:5: new(FooMessage) escapes to heap

Notice how (*fooMessage).Type is inlineable, but the call to it on line 6 is not inlined? As a result, you see that new(FooMessage) escapes to the heap.

It seems that the cause of this is because the (*FooMessage).ProtoReflect function returns an interface. However, at the point when inlining happened, it should know that it has a concrete *fooMessage type instead of any arbitrary Message interface type. Knowing that it has a concrete *fooMessage type, then the call to Type should also be inlined.

If you change the Message return type to *fooMessage, then this inlines as expected:

./main.go:11:6: can inline (*FooMessage).ProtoReflect
./main.go:17:6: can inline (*fooMessage).Type
./main.go:6:30: inlining call to (*FooMessage).ProtoReflect
./main.go:6:37: inlining call to (*fooMessage).Type
./main.go:6:5: main new(FooMessage) does not escape

Furthermore, new(FooMessage) on line 6 no longer escapes.

@dsnet dsnet added the Performance label Jun 7, 2018
@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Jun 8, 2018

Notice how (*fooMessage).Type is inlineable, but the call to it on line 6 is not inlined? As a result, you see that new(FooMessage) escapes to the heap.

new(FooMessage) escaping has nothing to do with inlining. It has to do with being passed to an interface. Escape analysis gives up at that point.

I think devirtualization would fix this. That's issue #19361.
This issue is a little bit trickier than the example in #19361 because the object needs to be tracked through a callee.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Jun 8, 2018

I think devirtualization would fix this. That's issue #19361.

Closing as a dup of #19361. Please feel free to reopen if you think there is something we can and should do separately.

@bcmills bcmills closed this Jun 8, 2018
@golang golang locked and limited conversation to collaborators Jun 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.