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: range loop with large element has a significant cost #59706

Open
egonelbre opened this issue Apr 19, 2023 · 1 comment
Open

cmd/compile: range loop with large element has a significant cost #59706

egonelbre opened this issue Apr 19, 2023 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@egonelbre
Copy link
Contributor

egonelbre commented Apr 19, 2023

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

$ go version
go version devel go1.21-969ab34e46 Wed Apr 19 02:10:00 2023 +0000 windows/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Here's a benchmark of two approaches of writing slices.Index:

package main

import "testing"

type Large [4 * 1024]byte

const N = 1024

func IndexA(s []Large, v Large) int {
	for i, vs := range s {
		if v == vs {
			return i
		}
	}
	return -1
}

func IndexB(s []Large, v Large) int {
	for i := range s {
		if v == s[i] {
			return i
		}
	}
	return -1
}

func BenchmarkA_Large(b *testing.B) {
	elements := make([]Large, N)
	for k := 0; k < b.N; k++ {
		IndexA(elements, Large{1})
	}
}

func BenchmarkB_Large(b *testing.B) {
	elements := make([]Large, N)
	for k := 0; k < b.N; k++ {
		IndexB(elements, Large{1})
	}
}

Results:

BenchmarkA_Large-32         3129            352215 ns/op
BenchmarkB_Large-32       256124              4516 ns/op

I would have expected the results to be approximately the same, however, there's a significant cost to using for i, v := range s loop.

As far as I was able to tell, the slower implementation ends up making more copies of the v.

I made a quick fix for slices package for the time being https://go-review.googlesource.com/c/go/+/486235.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 19, 2023
@egonelbre egonelbre changed the title cmd/compile: range loop has a significant cost cmd/compile: generic range loop with large element has a significant cost Apr 19, 2023
@egonelbre egonelbre changed the title cmd/compile: generic range loop with large element has a significant cost cmd/compile: range loop with large element has a significant cost Apr 19, 2023
@prattmic
Copy link
Member

cc @golang/compiler @dr2chase

@prattmic prattmic added this to the Backlog milestone Apr 21, 2023
@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
Development

No branches or pull requests

4 participants