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: read-only escape analysis and avoiding string -> []byte copies #2205
Comments
Comment 1 by jp@webmaster.ms: It seem to be much easier to add the possibility of zerocopish taking string slices from byte slices/arrays. As far I understand, string slices are actually readonly and cap()'less byte slices: http://research.swtch.com/2009/11/go-data-structures.html |
Owner changed to @rsc. Status changed to Accepted. |
I run into this often, but I should start listing examples. In goprotobuf, text.go calls writeString with both a string and a []byte converted to a string: // writeAny writes an arbitrary field. func writeAny(w *textWriter, v reflect.Value, props *Properties) { v = reflect.Indirect(v) // We don't attempt to serialise every possible value type; only those // that can occur in protocol buffers, plus a few extra that were easy. switch v.Kind() { case reflect.Slice: // Should only be a []byte; repeated fields are handled in writeStruct. writeString(w, string(v.Interface().([]byte))) case reflect.String: writeString(w, v.String()) Note that the function writeString reallly just wants a read-only slice of bytes: func writeString(w *textWriter, s string) { w.WriteByte('"') // Loop over the bytes, not the runes. for i := 0; i < len(s); i++ { // Divergence from C++: we don't escape apostrophes. // There's no need to escape them, and the C++ parser // copes with a naked apostrophe. switch c := s[i]; c { case '\n': w.Write([]byte{'\\', 'n'}) case '\r': w.Write([]byte{'\\', 'r'}) case '\t': w.Write([]byte{'\\', 't'}) case '"': w.Write([]byte{'\\', '"'}) case '\\': w.Write([]byte{'\\', '\\'}) default: if isprint(c) { w.WriteByte(c) } else { fmt.Fprintf(w, "\\%03o", c) } } } w.WriteByte('"') } It doesn't matter that it's frozen (like a string), nor writable (like a []byte). But Go lacks that type, so if instead it'd be nice to write writeAny with a []byte parameter and invert the switch above to be like: switch v.Kind() { case reflect.Slice: // Should only be a []byte; repeated fields are handled in writeStruct. writeString(w, v.Interface().([]byte)) case reflect.String: writeString(w, []byte(v.String())) // no copy! Where the []byte(v.String()) just makes a slice header pointing in to the string's memory, since the compiler can verify that writeAny never mutates its slice. |
See patch and CL description at http://golang.org/cl/6850067 for the opposite but very related case: strconv.ParseUint, ParseBool, etc take a string but calling code has a []byte. |
People who like this bug also like issue #3512 (cmd/gc: optimized map[string] lookup from []byte key) |
FWIW var m map[string]int var b []byte _ = m[string(b)] case must be radically simpler to implement than general read-only analysis. It's peephole optimization, when the compiler sees such code it can generate hacky string object using the []byte pointer. Another example would be len(string(b)), but this seems useless. |
Yes. That's why they're separate bugs. I want issue #3512 first, because it's easy. |
As my understanding, []byte -> string conversion don't require read-only checks. It require only escape analysis that current Go has. Am I correct? We have several duplicated functions for bytes and strings. Implementing only []byte -> string conversion first would very helpful for us. func escapeString(in string) []byte {
// some escape logic here.
// do not escape *in*
}
func escapeBytes(in []byte) []byte {
return escapeString(string(in)) // slicebytetostringtmp, instead of slicebytetostring.
} |
No, at least if I understand you correctly. We need to handle this case:
The string So it isn't that In any case, this issue is for the conversion the other way, string -> []byte. |
Thank you. I thought about string() in argument list like
Oh, I didn't know that. Would you change the issue title from |
IMO, I think that it's difficult to provide way for converting types since that bytes might be on stack not heap allocator. var s string
// s = "hello" // This should be crash because "hello" might be on stack.
s = string([]byte("hello"))
header := (*reflect.StringHeader)(unsafe.Pointer(&s))
bytesheader := &reflect.SliceHeader{
Data: header.Data,
Len: header.Len,
Cap: header.Len,
}
b := *(*[]byte)(unsafe.Pointer(bytesheader))
b[0] = 0x48 |
Simpler example (tested with Go 1.17.8 / 1.18): package pkg
// ToLower converts ascii string s to lower-case
func ToLower(s string) string {
if s == "" {
return ""
}
buf := make([]byte, len(s))
for i := 0; i < len(s); i++ {
c := s[i]
if 'A' <= c && c <= 'Z' {
c |= 32
}
buf[i] = c
}
return string(buf)
} pkg_test.go package pkg
import "testing"
func BenchmarkToLower(b *testing.B) {
str := "SomE StrInG"
want := "some string"
var res string
for i := 0; i < b.N; i++ {
res = ToLower(str)
}
if res != want {
b.Fatal("ToLower error")
}
}
There is a redundant allocation & copy in |
Change https://go.dev/cl/520259 mentions this issue: |
Change https://go.dev/cl/520395 mentions this issue: |
This CL extends escape analysis in two ways. First, we already optimize directly called closures. For example, given: var x int // already stack allocated today p := func() *int { return &x }() we don't need to move x to the heap, because we can statically track where &x flows. This CL extends the same idea to work for indirectly called closures too, as long as we know everywhere that they're called. For example: var x int // stack allocated after this CL f := func() *int { return &x } p := f() This will allow a subsequent CL to move the generation of go/defer wrappers earlier. Second, this CL adds tracking to detect when pointer values flow to the pointee operand of an indirect assignment statement (i.e., flows to p in "*p = x") or to builtins that modify memory (append, copy, clear). This isn't utilized in the current CL, but a subsequent CL will make use of it to better optimize string->[]byte conversions. Updates #2205. Change-Id: I610f9c531e135129c947684833e288ce64406f35 Reviewed-on: https://go-review.googlesource.com/c/go/+/520259 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
CL 520395 that resolved this issue was reverted in CL 520596, so reopening. |
Change https://go.dev/cl/520599 mentions this issue: |
Change https://go.dev/cl/520600 mentions this issue: |
Hi, |
@jfcg No, at this time only string->[]byte. |
This CL implements the remainder of the zero-copy string->[]byte conversion optimization initially attempted in go.dev/cl/520395, but fixes the tracking of mutations due to ODEREF/ODOTPTR assignments, and adds more comprehensive tests that I should have included originally. However, this CL also keeps it behind the -d=zerocopy flag. The next CL will enable it by default (for easier rollback). Updates #2205. Change-Id: Ic330260099ead27fc00e2680a59c6ff23cb63c2b Reviewed-on: https://go-review.googlesource.com/c/go/+/520599 Auto-Submit: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The optimization is cool, but it is a little pity that the following conversions still need allocations. func CompareStrings(a, b string) int {
return bytes.Compare([]byte(a), []byte(b))
} package main
import "bytes"
import t "testing"
var y = "abcdefghijklmnopqrstuvwxyz0123456789"
func compareNonConstants() {
_ = bytes.Compare([]byte(y), []byte(y))
}
func main() {
stat := func(f func()) int {
allocs := t.AllocsPerRun(10, f)
return int(allocs)
}
println(stat(compareNonConstants)) // 2
} package main
import t "testing"
var y = "abcdefghijklmnopqrstuvwxyz0123456789"
var s = []byte(y)
func concatBytesAndString() {
_ = append([]byte(y), s...)
}
func main() {
stat := func(f func()) int {
allocs := t.AllocsPerRun(10, f)
return int(allocs)
}
println(stat(concatBytesAndString)) // 2
} |
BTW, should the old optimization for |
The text was updated successfully, but these errors were encountered: