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: statically remove type switch in instantiated generic code #57072

Open
dsnet opened this issue Dec 4, 2022 · 2 comments
Open

cmd/compile: statically remove type switch in instantiated generic code #57072

dsnet opened this issue Dec 4, 2022 · 2 comments
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

@dsnet
Copy link
Member

dsnet commented Dec 4, 2022

Using Go1.19

Consider this benchmark, which has three functions:

  • lessUTF16 is a generic function that operates on either []byte or string. It relies on a type switch to call either utf8.DecodeRune or utf8.DecodeRuneInString.
  • lessUTF16Bytes is identical to lessUTF16 but is manually instantiated for []byte.
  • lessUTF16String is identical to lessUTF16 but is manually instantiated for string.

The diff from lessUTF16 to lessUTF16Bytes is:

-		var rx, ry rune
-		var nx, ny int
-		switch any(x).(type) {
-		case string:
-			rx, nx = utf8.DecodeRuneInString(string(x))
-			ry, ny = utf8.DecodeRuneInString(string(y))
-		case []byte:
-			rx, nx = utf8.DecodeRune([]byte(x))
-			ry, ny = utf8.DecodeRune([]byte(y))
-		}
+		rx, nx := utf8.DecodeRune(x)
+		ry, ny := utf8.DecodeRune(y)

Similarly, the diff from lessUTF16 to lessUTF16String is:

-		var rx, ry rune
-		var nx, ny int
-		switch any(x).(type) {
-		case string:
-			rx, nx = utf8.DecodeRuneInString(string(x))
-			ry, ny = utf8.DecodeRuneInString(string(y))
-		case []byte:
-			rx, nx = utf8.DecodeRune([]byte(x))
-			ry, ny = utf8.DecodeRune([]byte(y))
-		}
+		rx, nx := utf8.DecodeRuneInString(x)
+		ry, ny := utf8.DecodeRuneInString(y)

I would expect lessUTF16[[]byte] to perform identically to lessUTF16Bytes and lessUTF16[string] to perform identically to lessUTF16String since the compiler can prove that only one of the cases are possibly executable. However, that is not what the benchmarks show:

BenchmarkString           	 1752459	       686.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkBytes            	 1573508	       765.1 ns/op	       0 B/op	       0 allocs/op
BenchmarkGenericString    	 1595708	       744.2 ns/op	       0 B/op	       0 allocs/op
BenchmarkGenericBytes     	 1527207	       793.7 ns/op	       0 B/op	       0 allocs/op

\cc @randall77

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 4, 2022
@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 5, 2022
@prattmic
Copy link
Member

prattmic commented Dec 5, 2022

cc @golang/compiler

@prattmic prattmic modified the milestone: Backlog Dec 5, 2022
@cuonglm
Copy link
Member

cuonglm commented Dec 13, 2022

I think it's not the type switch, but the extra cost comes from the tail call in the wrapper function to the shape version. For example, lessUTF16[string] is roughly compiled to:

func lessUTF16[string](x, y string) bool {
    return lessUTF16[go.shape.string](.dict.lessUTF16[string], x, y)
}

cc @mdempsky

@prattmic prattmic added this to the Backlog milestone Dec 14, 2022
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
Status: Todo
Development

No branches or pull requests

5 participants