-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: opt: eliminate unnecessary slicebytetostring call in argument to mapaccess1_faststr #71132
Comments
Related Issues
Related Code Changes Related Documentation (Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
cc @golang/compiler |
I believe this optimization is occurring at https://cs.opensource.google/go/go/+/master:src/cmd/compile/internal/walk/order.go;l=337;drc=705b5a569acb9207fda8ea469387d88346b6817a;bpv=1;bpt=1 in walk. walk is after inlining, so that isn't quite the problem. I wonder if the issue is simply that this is looking for an OBYTES2STR node, but the inlined call would be an OINLCALL node, which itself contains the OBYTES2STR in its body. |
FWIW, Keith mentioned in #44898 (comment) for a similar issue:
I was wondering if this might be addressable in escape analysis to make it a zero-copy []byte->string conversion , and was taking a quick look at it. (zero-copy string->[]byte conversion exists today in escape analysis, but not the reverse; there are a couple of issues about that). |
Change https://go.dev/cl/640656 mentions this issue: |
In the
g
function below, a map lookupm[string(byteslice)]
avoids allocating a string, presumably because the compiler knows that the use is non-escaping and non-mutating; but if the string conversion is buried within an inlined function, as in thef
example below, the optimization is defeated.Could the optimization be applied after inlining? It's quite common for the result of a String() method to be used as hash key, and the strings are often constructed from a byte slice (perhaps via a strings.Builder or bytes.Buffer).
The text was updated successfully, but these errors were encountered: