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: potential deadcode elimination opportunity of unused methods #38685

Closed
typeless opened this issue Apr 27, 2020 · 10 comments
Closed

cmd/link: potential deadcode elimination opportunity of unused methods #38685

typeless opened this issue Apr 27, 2020 · 10 comments

Comments

@typeless
Copy link

@typeless typeless commented Apr 27, 2020

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

$ go version
go version go1.14.2 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
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/src/golang.org/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/src/golang.org/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build436707887=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/lNId1c5z_Dk

cat <<EOF > hello.go
package main

import (
        "fmt"
        "io"
)

type Bar interface {
        Bark()
        Meow()
}

type Foo struct {
}

func (*Foo) Bark() {
        fmt.Println("Bark")
}
func (*Foo) Meow() {
        fmt.Println("Meow")
}

func baz(v Bar) {
        v.Bark()
}

func main() {
        x := &Foo{}
        baz(x)
}
EOF
$ go build hello.go
$ nm hello | grep Meow
00000000004915c0 T main.(*Foo).Meow
00000000004da690 R main.(*Foo).Meow.stkobj

What did you expect to see?

nm hello | grep Meow

should output nothing.

What did you see instead?

$ nm hello | grep Meow
00000000004915c0 T main.(*Foo).Meow
00000000004da690 R main.(*Foo).Meow.stkobj

Gccgo has the remaining Meow too. But tinygo successfully removes the method.

Related to #6853 and #36313

Also found https://aykevl.nl/2018/12/tinygo-interface, which might be of interest.

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Apr 27, 2020

We probably could do that. I'm thinking on reworking the linker's live method analysis. I'll keep this in mind.

I think the compiler needs to emit some kind of marker at Bar.Bark interface call, so the linker knows Bark is used. The linker then could track only marked interface methods, instead of all of them as we are doing today.

The question is how frequently this will fire, i.e. how many methods we can eliminate by doing this.

But, most importantly, as a cat person, I oppose removing the Meow method regardless.

@cherrymui cherrymui changed the title potential deadcode elimination opportunity of unused methods cmd/link: potential deadcode elimination opportunity of unused methods Apr 27, 2020
@cherrymui cherrymui added this to the Unplanned milestone Apr 27, 2020
@typeless
Copy link
Author

@typeless typeless commented Apr 28, 2020

Also for the following example, I expect the Fish's Meow and Bark should be removed.

package main

import (
        "fmt"
)

type Bar interface {
        Bark()
        Meow()
}

type Foo struct {
}

func (*Foo) Bar() {
        fmt.Println("Bar")
}

//go:noinline
func (f *Foo) Bark() {
        fmt.Println("Meow")
}

//go:noinline
func (*Foo) Meow() {
        fmt.Println("Meow")
}

type Fish struct {
}

//go:noinline
func (*Fish) Bark() {
        fmt.Println("Fish Bark")
}

//go:noinline
func (*Fish) Meow() {
        fmt.Println("Fish Meow")
}

func baz(v Bar) {
        v.Bark()
}

func main() {
        x := &Foo{}
        y := &Fish{}
        baz(x)
        fmt.Println(x)
        fmt.Println(y)
}
$ nm hello | grep Fish
0000000000491650 T main.(*Fish).Bark
00000000004da760 R main.(*Fish).Bark.stkobj
00000000004916e0 T main.(*Fish).Meow
00000000004da780 R main.(*Fish).Meow.stkobj
@bradfitz bradfitz added the binary-size label May 1, 2020
@rojer
Copy link

@rojer rojer commented Aug 19, 2020

We at u-root are very interested in this.
DCE that eliminates exported non-interface methods saves about 12% for us (and when it stops working our binaries no longer fit into flash chips which kinda sucks).
I expect eliminating unused interface methods to bring similar improvement.
@cherrymui If you can put together a patch I will be happy to test it to give you some numbers.

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Aug 19, 2020

@rojer Have you tried the tip version of Go? I made some improvements in the dev.link branch which is merged a few days ago. Does that make any difference? Thanks.

@rojer
Copy link

@rojer rojer commented Aug 19, 2020

@cherrymui looks like there's been a minor improvement since 1.15:

[rojer@nbd ~/go/src/github.com/u-root/u-root master]$ for GO in ~/go/go1.14.7/bin/go ~/go/go1.15/bin/go ~/go/go-git/bin/go; do $GO version; $GO run github.com/u-root/u-root/tools/makebb -o /tmp/bb cmds/*/* > /dev/null 2>&1; ls -la /tmp/bb; done
go version go1.14.7 linux/amd64
-rwxrwxr-x 1 rojer rojer 17956864 Aug 19 20:11 /tmp/bb
go version go1.15 linux/amd64
-rwxrwxr-x 1 rojer rojer 14299136 Aug 19 20:11 /tmp/bb
go version devel +64350f1eab Wed Aug 19 17:54:18 2020 +0000 linux/amd64
-rwxrwxr-x 1 rojer rojer 14127104 Aug 19 20:11 /tmp/bb

big step down from 1.14 is the unused methods DCE: 1.15 shipped with the fix for #36021

@thanm
Copy link
Member

@thanm thanm commented Nov 5, 2020

This looks to be fixed on tip:

$ go build barkmeow.go
$ nm barkmeow | fgrep Meow
$ 

Marking this bug closed -- please reopen if you disagree. Thanks.

@thanm thanm closed this Nov 5, 2020
@rojer
Copy link

@rojer rojer commented Nov 6, 2020

doesn't seem to work across package boundaries, UnusedInterfaceMethod doesn't get eliminated:

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

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")
}
@rojer
Copy link

@rojer rojer commented Nov 6, 2020

@thanm should i file a new issue for this?

@thanm
Copy link
Member

@thanm thanm commented Nov 6, 2020

Yes, please file a new issue. There are many different flavors/permutations/combinations when it comes to unused method removal; the details matter.

@rojer
Copy link

@rojer rojer commented Nov 6, 2020

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
6 participants
You can’t perform that action at this time.