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/link: Unused interface methods don't get gc'd #42421

Open
rojer opened this issue Nov 6, 2020 · 4 comments
Open

cmd/link: Unused interface methods don't get gc'd #42421

rojer opened this issue Nov 6, 2020 · 4 comments
Labels
Milestone

Comments

@rojer
Copy link

@rojer rojer commented Nov 6, 2020

Follow up to #38685 that seems to have solved a simple case but does not eliminate an unused interface method across packages

What version of Go are you using (go version)?

[rojer@nbd /tmp]$ GOROOT=/home/rojer/go/go-git /home/rojer/go/go-git/bin/go version
go version devel +b7e0adfee2 Fri Nov 6 07:55:52 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
[rojer@nbd /tmp]$ GOROOT=/home/rojer/go/go-git /home/rojer/go/go-git/bin/go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rojer/.cache/go-build"
GOENV="/home/rojer/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/rojer/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/rojer/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/rojer/go/go-git"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/rojer/go/go-git/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build351127229=/tmp/go-build -gno-record-gcc-switches"

What did you do?

source:

[rojer@nbd /tmp]$ find foobar/
foobar/
foobar/go.mod
foobar/foo
foobar/foo/foo.go
foobar/bar
foobar/bar/bar.go
[rojer@nbd /tmp]$ cat foobar/foo/foo.go
package main

import (
        "foobar/bar"
)

func main() {
        // Test unused method elimination.
        var b bar.Interface
        b = bar.Bar{}
        b.UsedInterfaceMethod()
}
[rojer@nbd /tmp]$ cat foobar/bar/bar.go 
package bar

import "fmt"

type Interface interface {
        UsedInterfaceMethod()
        UnusedInterfaceMethod()
}

type Bar struct{}

func (Bar) UsedInterfaceMethod() {
        fmt.Println("one")
}

// This method is unused but cannot be eliminated yet.
// https://github.com/golang/go/issues/38685
func (Bar) UnusedInterfaceMethod() {
        fmt.Println("two")
}

// This method is unused and should be eliminated.
func (Bar) UnusedNonInterfaceMethod() {
        fmt.Println("three")
}

What did you expect to see?

foobar/bar.(*Bar).UnusedInterfaceMethod should not make it into the final binary.
in fact, neither should foobar/bar.(*Bar).UsedInterfaceMethod because pointer variant is not used.

in other words, all that should be left from bar.Bar should be bar.Bar.UsedInterfaceMethod.

interesting, that foobar/bar.Bar.UnusedInterfaceMethod does get eliminated but not the bar.*Bar variant.
why is it even generated in the first place?

What did you see instead?

[rojer@nbd /tmp/foobar/foo]$ GOROOT=/home/rojer/go/go-git /home/rojer/go/go-git/bin/go build 
[rojer@nbd /tmp/foobar/foo]$ GOROOT=/home/rojer/go/go-git /home/rojer/go/go-git/bin/go tool nm -size foo | grep foobar/bar
  497080        187 T foobar/bar.(*Bar).UnusedInterfaceMethod
  497140        187 T foobar/bar.(*Bar).UsedInterfaceMethod
  535e20         32 D foobar/bar..inittask
  496fe0        138 T foobar/bar.Bar.UsedInterfaceMethod
  4da288         40 R go.itab.foobar/bar.Bar,foobar/bar.Interface
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Nov 6, 2020

Thanks. Interesting example.

I think the unused method is linked in because it is referenced from the itab.

If you change foo.go to do

b = interface{}(bar.Bar{}).(bar.Interface)

then UnusedInterfaceMethod disappears.

I agree this is awkward, and we probably could fix the itab case. But it's too late for 1.16. Will try in 1.17. Thanks.

@toothrot toothrot added this to the Backlog milestone Nov 6, 2020
@zephyrtronium
Copy link
Contributor

@zephyrtronium zephyrtronium commented Nov 7, 2020

I'm surprised there is any case in which an exported method is eliminated from a compiled binary. Reflection can reach it without ever naming it: https://play.golang.org/p/zCIdBYzv8zo. I think that's the reason the (Bar) version is eliminated but the (*Bar) version isn't. The non-pointer method is never used, but the pointer-shaped wrapper could be needed through an interface, especially by reflection. (If I'm wrong, please correct me. I'm still learning the internals of cmd/compile.)

@rojer
Copy link
Author

@rojer rojer commented Nov 7, 2020

@zephyrtronium unused method elimination is disabled if use of reflection is detected. the approach to dead code elimination is described here.
projects that care about binary size have to avoid using reflection in that way (and we do).

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 9, 2020

Change https://golang.org/cl/268479 mentions this issue: cmd/compile, cmd/link: use weak reference in itab

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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