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: optimize switch string(byteSlice) lookups #24937

Closed
dchenk opened this issue Apr 18, 2018 · 4 comments

Comments

Projects
None yet
5 participants
@dchenk
Copy link
Contributor

commented Apr 18, 2018

I see this pattern commonly:

switch string(byteSlice) {
case "some_key":
    // Stuff ...
case "another":
    // Stuff ...
// More cases ...
}

(Instances of this pattern exist in the standard library.)

Here an allocation is made for the string, copying the contents of the []byte. It'd be great if the contents of the slice could be used without an allocation of a string, using simply a string header pointing to the same location as the slice. This will avoid creating a ton of garbage.

This is the same kind of proposal as #3512 by @bradfitz (and I think the implementation would look similar).

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 18, 2018

The compiler already implements string(byteSlice) == "string literaal" without an allocation.

So I'm a little surprised that we don't get this for free right now. Maybe if switch were rewritten to a series of if statements then we'd get it for free?

/cc @mdempsky @josharian

@bradfitz bradfitz added this to the Unplanned milestone Apr 18, 2018

@josharian

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2018

Maybe if switch were rewritten to a series of if statements then we'd get it for free?

We would, but we'd lose the nice binary search properties. This should be an easy fix. I'll look soon.

@mdempsky

This comment has been minimized.

Copy link
Member

commented Apr 18, 2018

The string(byteSlice) == "string literal" optimization doesn't extend to switch statements because we're actually doing:

tmp := string(byteSlice)
if tmp == "some_key" { ... }
else if tmp == "another" { ... }

(And actually more complicated, because we handle runs of constants with binary search.)

One easy optimization we could do here is that in swt.go, if s.exprname.Op is OARRAYBYTESTR and all of the case value expressions are side-effect free (at least no function calls or channel receives), we can rewrite the Op to ARRAYBYTESTRTMP.

It's important that we check for side-effects, because otherwise we'd miscompile expressions like:

x := []byte{'a'}
switch string(x) {
case func() string { x[0] = 'b'; return "b" }():
    panic("FAIL")
}
@gopherbot

This comment has been minimized.

Copy link

commented Apr 21, 2018

Change https://golang.org/cl/108035 mentions this issue: cmd/compile: avoid runtime call during switch string(byteslice)

@gopherbot gopherbot closed this in 566e3e0 Apr 21, 2018

@golang golang locked and limited conversation to collaborators Apr 21, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.