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: Go 1.19 might make generic types slower #54238

Open
kscooo opened this issue Aug 3, 2022 · 3 comments
Open

cmd/compile: Go 1.19 might make generic types slower #54238

kscooo opened this issue Aug 3, 2022 · 3 comments
Assignees
Labels
compiler/runtime NeedsInvestigation Performance
Milestone

Comments

@kscooo
Copy link

@kscooo kscooo commented Aug 3, 2022

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

$ go version
go version go1.19 linux/amd64

Does this issue reproduce with the latest release?

It reproduces on the 1.19

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ksco/.cache/go-build"
GOENV="/home/ksco/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ksco/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ksco/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3292360144=/tmp/go-build -gno-record-gcc-switches"

What did you do?

playground: https://go.dev/play/p/6UxOn6pftVs

main_test.go

package main

import (
	"testing"
)

func addInt(a, b int) int {
	return a + b
}

func addString(a, b string) string {
	return a + b
}

func BenchmarkWithoutGenerics(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_ = addInt(1, 2)
		_ = addString("foo", "bar")
	}
}

type Addable interface {
	int | string
}

func add[T Addable](a, b T) T {
	return a + b
}

func BenchmarkWithGenerics(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_ = add(1, 2)
		_ = add("foo", "bar")
	}
}

What did you expect to see?

1.19 should be similar to the 1.18 benchmark test, not much slower.

What did you see instead?

goversion: 1.19

goos: linux
goarch: amd64
pkg: generics/ch13
cpu: AMD Ryzen 7 5800H with Radeon Graphics         
BenchmarkWithoutGenerics
BenchmarkWithoutGenerics-16    	71445846	        16.49 ns/op
BenchmarkWithGenerics
BenchmarkWithGenerics-16       	32776173	        36.30 ns/op
PASS

goversion: 1.18

goos: linux
goarch: amd64
pkg: generics/ch13
cpu: AMD Ryzen 7 5800H with Radeon Graphics         
BenchmarkWithoutGenerics
BenchmarkWithoutGenerics-16    	64931702	        16.91 ns/op
BenchmarkWithGenerics
BenchmarkWithGenerics-16       	70497928	        17.01 ns/op
PASS

The assembly generated by 1.19 is found to have more

main..dict.add[int](SB), AX
main..dict.add[string](SB), AX

These lines are directly inlined in 1.18 (there may be a mistake in this expression, forgive me for not being familiar with the principles of assembly).

/cc @ianlancetaylor

@gopherbot gopherbot added the compiler/runtime label Aug 3, 2022
@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Aug 3, 2022

does export GOEXPERIMENT=unified make it better?

@dmitshur dmitshur added Performance NeedsInvestigation labels Aug 3, 2022
@randall77
Copy link
Contributor

@randall77 randall77 commented Aug 3, 2022

This was caused by https://go-review.googlesource.com/c/go/+/395854 . It was a deliberate change to avoid inlining in cases where we were running into bugs.
I think we will leave this as is for 1.19. unified IR should fix this for 1.20.

@randall77 randall77 added this to the Go1.20 milestone Aug 3, 2022
@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Aug 3, 2022

Since unified IR is expected to fix this, I'm going to assign this @mdempsky for visibility, but feel free to unassign.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime NeedsInvestigation Performance
Projects
Status: Todo
Development

No branches or pull requests

7 participants