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: reduced microbenchmark execution speed in 1.17 #47746

Closed
0x1BB736F opened this issue Aug 17, 2021 · 2 comments
Closed

cmd/compile: reduced microbenchmark execution speed in 1.17 #47746

0x1BB736F opened this issue Aug 17, 2021 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@0x1BB736F
Copy link

0x1BB736F commented Aug 17, 2021

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

$ go version
go version go1.17 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
GOARCH="amd64"
GOOS="linux"
CGO_ENABLED="0"

cpu: Intel(R) Core(TM) i5-9400 CPU @ 2.90GHz

What did you do?

I ran the benchmark in versions 1.16.4, 1.15.15 and 1.17.0 and saw a decrease in the execution speed

func testFnInt(x, y int) {
	v := x + y
	x = x + y
	y = v * x
}

func testSlice(v []int) {
	for id, val := range v {
		testFnInt(id, val)
	}
}

func Benchmark_IntF(b *testing.B) {
	v := []int{11, 22, 55, 1, -11, 0, 22}

	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		testSlice(v)
	}
}

go test -benchmem -bench=. test/bench_test.go

What did you expect to see?

no difference or very minimal difference at all

What did you see instead?

1.15.15

Benchmark_IntF-6        322077016                3.57 ns/op            0 B/op          0 allocs/op
Benchmark_IntF-6        325528543                3.66 ns/op            0 B/op          0 allocs/op
Benchmark_IntF-6        325288412                3.72 ns/op            0 B/op          0 allocs/op
Benchmark_IntF-6        329647598                3.70 ns/op            0 B/op          0 allocs/op
Benchmark_IntF-6        326699432                3.71 ns/op            0 B/op          0 allocs/op

1.16.4

Benchmark_IntF-6   	328683681	         3.674 ns/op	       0 B/op	       0 allocs/op
Benchmark_IntF-6   	329477650	         3.658 ns/op	       0 B/op	       0 allocs/op
Benchmark_IntF-6          320231263                3.666 ns/op           0 B/op             0 allocs/op
Benchmark_IntF-6          330243132                3.636 ns/op           0 B/op             0 allocs/op
Benchmark_IntF-6          326361471                3.663 ns/op           0 B/op          0 allocs/op

1.17.0

Benchmark_IntF-6   	290349687	         4.114 ns/op	       0 B/op	       0 allocs/op
Benchmark_IntF-6   	290900883	         4.116 ns/op	       0 B/op	       0 allocs/op
Benchmark_IntF-6   	290776753	         4.114 ns/op	       0 B/op	       0 allocs/op
Benchmark_IntF-6   	290126416	         4.136 ns/op	       0 B/op	       0 allocs/op
Benchmark_IntF-6   	289591995	         4.171 ns/op	       0 B/op	       0 allocs/op
@0x1BB736F 0x1BB736F changed the title increased execution speed in 1.17 reduced execution speed in 1.17 Aug 17, 2021
@mknyszek mknyszek changed the title reduced execution speed in 1.17 cmd/compile: reduced microbenchmark execution speed in 1.17 Aug 17, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 17, 2021
@mknyszek mknyszek added this to the Go1.17.1 milestone Aug 17, 2021
@mknyszek
Copy link
Contributor

Looking at your benchmark, it appears to me that it isn't doing anything. I suspect that the compiler can pretty much optimize everything away, because there's no result being propagated out of testFnInt. This happens even for 1.16. See https://godbolt.org/z/4W369Mz9E.

As a result, I'm not sure this is a case worth worrying about. This difference in performance could come from just about anything, including a slightly different alignment of the assembly code for testSlice and Benchmark_IntF (which appear to still be there). The compiler changed a good bit in the 1.17 release (see https://golang.org/doc/go1.17), so changes like this are inevitable.

Closing this issue as a result.

@dmitshur
Copy link
Contributor

dmitshur commented Sep 1, 2021

This issue was closed in NeedsInvestigation state without a fix, but if a fix were to be made, it would need to target Go 1.18 first. Since this isn't a backport issue, moving this out of Go1.17.1 milestone to help keep the 1.17.1 milestone more focused on changes going into that upcoming minor release.

@dmitshur dmitshur modified the milestones: Go1.17.1, Go1.18 Sep 1, 2021
@golang golang locked and limited conversation to collaborators Sep 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants