Many functions take a []byte but only read from it. If the escape analysis code could flag parameters of a function as read-only, then code which passes in a []byte(string) conversion could be cheaper. bytes.IndexByte is one example. For example, http/sniff.go does: // ------------------- // Index of the first non-whitespace byte in data. firstNonWS := 0 for ; firstNonWS < len(data) && isWS(data[firstNonWS]); firstNonWS++ { } func isWS(b byte) bool { return bytes.IndexByte([]byte("\t\n\x0C\n "), b) != -1 } // ------------------- But it's faster to avoid re-creating the []byte: func BenchmarkIndexByteLiteral(b *testing.B) { for i := 0; i < b.N; i++ { IndexByte([]byte("\t\n\x0C\n "), 'x') } } func BenchmarkIndexByteVariable(b *testing.B) { var whitespace = []byte("\t\n\x0C\n ") for i := 0; i < b.N; i++ { IndexByte(whitespace, 'x') } } bytes_test.BenchmarkIndexByteLiteral 20000000 125 ns/op bytes_test.BenchmarkIndexByteVariable 100000000 25.4 ns/op Related is issue #2204.
Comment From: gopherbot
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
Comment From: bradfitz
zerocopish?
Comment From: rsc
Can you be more specific? Examples of programs that you think should be handled specially would be a good way to do that.
Comment From: rsc
Owner changed to @rsc.
Status changed to Accepted.
Comment From: lvdlvd
Labels changed: added compilerbug, performance.
Comment From: rsc
Labels changed: added priority-later.
Comment From: rsc
Labels changed: added go1.1maybe.
Comment From: bradfitz
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.
Comment From: bradfitz
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.
Comment From: robpike
Labels changed: removed go1.1maybe.
Comment From: rsc
[The time for maybe has passed.]
Comment From: bradfitz
People who like this bug also like issue #3512 (cmd/gc: optimized map[string] lookup from []byte key)
Comment From: dvyukov
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.
Comment From: bradfitz
Yes. That's why they're separate bugs. I want issue #3512 first, because it's easy.
Comment From: bradfitz
Labels changed: added garbage.
Comment From: rsc
Labels changed: added priority-someday, removed priority-later.
Comment From: rsc
Labels changed: added repo-main.
Comment From: rsc
Adding Release=None to all Priority=Someday bugs.
Labels changed: added release-none.
Comment From: gopherbot
Change https://golang.org/cl/146018 mentions this issue: strings: declare Index as noescape
Comment From: josharian
I run into this often, but I should start listing examples.
Good idea. From #29802/#29810: encoding/hex.Decode.
Comment From: cespare
This comes up with hash functions (example: sha256.Sum256([]byte(s))
).
For github.com/cespare/xxhash in addition to Sum64 I added Sum64String which does the unsafe string-to-slice conversion:
https://github.com/cespare/xxhash/blob/3767db7a7e183c0ad3395680d460d0b61534ae7b/xxhash_unsafe.go#L28-L35
but I'd rather the caller just be able to use Sum64([]byte(s))
and have the compiler optimize it.
Comment From: zigo101
If read-only byte slice is supported, we can set the cap of read-only byte slices as a negative value to indicate the underlying bytes are immutable. []byte(aString)
results a read-only byte slice without duplicating the underlying bytes of aString
. Convertingthe result byte slice, if its cap is a negative value, back again to a string also needs not to duplicate the underlying bytes.
Without the read-only slice feature, is it possible to lazy duplicate the underlying bytes for coversion []byte(aString) on demand?
Comment From: ianlancetaylor
What I think might be useful:
- For every function/method that has a parameter of type
[]byte
, record whether the slice is modified.- Record this in the export data so that the information is correct interprocedurally.
- When calling a function with an argument of the form
[]byte(s)
for somes
of typestring
, if the function does not modify the[]byte
, don't copy the string. - For an interface with a method
M
that has a parameter of[]byte
, add an additional hidden methodMString
which has a parameter of typestring
.- Perhaps only for methods with a single
[]byte
parameter. - Perhaps wait until we have profile guided optimization and use that to see whether this is useful.
- Perhaps only for methods with a single
- When converting a type to this interface, if the type's
M
method does not modify the[]byte
, haveMString
forward toM
. - If the type's
M
method does modify the[]byte
, haveMString
copy to[]byte
and callM
. - Given an value of the interface type, when calling
M
with[]byte(s)
, callMString
instead.
Comment From: zigo101
For every function/method that has a parameter of type
[]byte
, record whether the slice is modified.
This might be hard to implement overall, mainly caused by the cases in which the []byte
parameter is passed to an interface method within the function body. We could think such functions will modify the slice parameters, but this will discount the usefulness of the idea much.
Comment From: bcmills
When converting a type to this interface, if the type's
M
method does not modify the[]byte
, haveMString
forward toM
.
Many io.Writer
implementations wrap one or more other io.Writer
implementations invoked somewhere in the method body. How would this forwarding logic work for those implementations?
Comment From: gopherbot
Change https://golang.org/cl/285681 mentions this issue: [dev.regabi] cmd/compile: optimize read-only []byte conversions
Comment From: mdempsky
Made some initial progress towards this in CL 285681. It needs polish and more tests, but it works well enough for Brad's initial test case.
One known issue at the moment is that it optimizes []byte("")
to []byte(nil)
instead of []byte{}
, so it could misbehave for programs that are sensitive to the difference. (I'll of course fix this before sending the CL for review.) But otherwise, I think it should work, and folks could test it out to see if there are notable cases where it helps or (even more critically) causes problems.
It currently finds 188 instances of string->[]byte conversions within GOROOT that can be converted without copying:
188 zero-copy byte-slice conversions from CL 285681
cmd/cgo/main.go:383:25: zero-copy byte-slice conversion
cmd/cgo/out.go:129:41: zero-copy byte-slice conversion
cmd/compile/internal/ssa/html.go:1186:38: zero-copy byte-slice conversion
cmd/compile/internal/syntax/scanner.go:424:19: zero-copy byte-slice conversion
cmd/cover/cover.go:170:13: zero-copy byte-slice conversion
cmd/dist/test.go:1646:26: zero-copy byte-slice conversion
cmd/dist/util.go:241:15: zero-copy byte-slice conversion
cmd/go/internal/bug/bug.go:176:15: zero-copy byte-slice conversion
cmd/go/internal/bug/bug.go:208:63: zero-copy byte-slice conversion
cmd/go/internal/cache/cache.go:375:24: zero-copy byte-slice conversion
cmd/go/internal/cache/default.go:51:52: zero-copy byte-slice conversion
cmd/go/internal/envcmd/env.go:507:15: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:269:19: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:269:19: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:283:36: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:283:36: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:289:19: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:289:19: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:323:36: zero-copy byte-slice conversion
cmd/go/internal/generate/generate.go:323:86: zero-copy byte-slice conversion
cmd/go/internal/modcmd/download.go:98:26: zero-copy byte-slice conversion
cmd/go/internal/modcmd/vendor.go:116:25: zero-copy byte-slice conversion
cmd/go/internal/modcmd/vendor.go:121:26: zero-copy byte-slice conversion
cmd/go/internal/modcmd/vendor.go:149:26: zero-copy byte-slice conversion
cmd/go/internal/modfetch/codehost/codehost.go:213:44: zero-copy byte-slice conversion
cmd/go/internal/modfetch/codehost/git.go:181:41: zero-copy byte-slice conversion
cmd/go/internal/modfetch/codehost/git.go:828:51: zero-copy byte-slice conversion
cmd/go/internal/modfetch/codehost/git.go:869:31: zero-copy byte-slice conversion
cmd/go/internal/modfetch/codehost/git.go:870:26: zero-copy byte-slice conversion
cmd/go/internal/test/test.go:1437:38: zero-copy byte-slice conversion
cmd/go/internal/test/test.go:1473:50: zero-copy byte-slice conversion
cmd/go/internal/trace/trace.go:147:26: zero-copy byte-slice conversion
cmd/go/internal/trace/trace.go:156:32: zero-copy byte-slice conversion
cmd/go/internal/version/exe.go:41:33: zero-copy byte-slice conversion
cmd/go/internal/version/exe.go:49:33: zero-copy byte-slice conversion
cmd/go/internal/version/exe.go:57:33: zero-copy byte-slice conversion
cmd/go/internal/version/exe.go:57:86: zero-copy byte-slice conversion
cmd/go/internal/web/api.go:224:55: zero-copy byte-slice conversion
cmd/go/internal/work/buildid.go:338:55: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:1471:38: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2332:35: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2372:52: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2374:34: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2392:67: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2589:42: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2590:30: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2591:30: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2592:30: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2874:34: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2878:47: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2884:41: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2895:52: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2899:50: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:2908:34: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:3098:35: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:3177:33: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:3177:78: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:640:37: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:640:77: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:641:35: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:641:75: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:642:35: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:642:76: zero-copy byte-slice conversion
cmd/go/internal/work/exec.go:96:39: zero-copy byte-slice conversion
cmd/internal/archive/archive.go:420:27: zero-copy byte-slice conversion
cmd/internal/buildid/rewrite.go:29:19: zero-copy byte-slice conversion
cmd/internal/codesign/codesign.go:245:26: zero-copy byte-slice conversion
cmd/internal/dwarf/dwarf.go:1628:33: zero-copy byte-slice conversion
cmd/internal/dwarf/dwarf.go:1635:36: zero-copy byte-slice conversion
cmd/internal/dwarf/dwarf.go:1636:40: zero-copy byte-slice conversion
cmd/internal/pkgpath/pkgpath.go:43:47: zero-copy byte-slice conversion
cmd/internal/pkgpath/pkgpath.go:56:31: zero-copy byte-slice conversion
cmd/internal/pkgpath/pkgpath.go:58:38: zero-copy byte-slice conversion
cmd/internal/pkgpath/pkgpath.go:60:38: zero-copy byte-slice conversion
cmd/link/internal/ld/data.go:2628:18: zero-copy byte-slice conversion
cmd/link/internal/ld/elf.go:1442:63: zero-copy byte-slice conversion
cmd/link/internal/ld/elf.go:1446:69: zero-copy byte-slice conversion
cmd/link/internal/ld/elf.go:593:18: zero-copy byte-slice conversion
cmd/link/internal/ld/elf.go:695:18: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1177:38: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1405:35: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1418:34: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1569:33: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1603:51: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1605:33: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1615:33: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1676:41: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1744:50: zero-copy byte-slice conversion
cmd/link/internal/ld/lib.go:1744:98: zero-copy byte-slice conversion
cmd/link/internal/ld/xcoff.go:1800:19: zero-copy byte-slice conversion
cmd/link/internal/loader/loader.go:2060:60: zero-copy byte-slice conversion
cmd/link/internal/loadpe/ldpe.go:366:84: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/internal/driver/fetch.go:524:22: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/internal/driver/options.go:63:23: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/internal/driver/options.go:91:15: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/internal/driver/webui.go:336:42: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/internal/driver/webui.go:336:56: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/internal/driver/webui.go:339:35: zero-copy byte-slice conversion
cmd/vendor/github.com/google/pprof/profile/legacy_java_profile.go:69:34: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:233:36: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:318:9: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:323:9: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:380:11: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:527:10: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:542:11: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:559:10: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:560:10: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:574:53: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:614:10: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:663:20: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:668:21: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:813:20: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:818:21: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/crypto/ssh/terminal/terminal.go:824:9: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/modfile/read.go:524:58: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:413:36: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:486:33: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:486:47: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:488:51: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:488:51: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:489:51: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/client.go:489:51: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/test.go:56:25: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/tlog/note.go:126:29: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/mod/sumdb/tlog/note.go:53:67: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/sys/unix/syscall_linux.go:976:26: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/sys/unix/syscall_linux.go:979:26: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/sys/unix/syscall_unix.go:355:29: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/tools/go/analysis/passes/buildtag/buildtag.go:112:34: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/tools/go/cfg/cfg.go:149:49: zero-copy byte-slice conversion
cmd/vendor/golang.org/x/tools/go/cfg/cfg.go:149:63: zero-copy byte-slice conversion
crypto/tls/handshake_messages.go:131:25: zero-copy byte-slice conversion
crypto/tls/handshake_messages.go:210:26: zero-copy byte-slice conversion
crypto/tls/handshake_messages.go:658:25: zero-copy byte-slice conversion
crypto/tls/handshake_messages.go:861:25: zero-copy byte-slice conversion
crypto/tls/key_schedule.go:39:20: zero-copy byte-slice conversion
crypto/tls/key_schedule.go:40:20: zero-copy byte-slice conversion
crypto/x509/x509.go:1809:24: zero-copy byte-slice conversion
crypto/x509/x509.go:1829:24: zero-copy byte-slice conversion
crypto/x509/x509.go:1841:24: zero-copy byte-slice conversion
encoding/base64/base64.go:385:35: zero-copy byte-slice conversion
encoding/xml/marshal.go:345:35: zero-copy byte-slice conversion
encoding/xml/xml.go:1091:23: zero-copy byte-slice conversion
go/internal/gccgoimporter/ar.go:85:66: zero-copy byte-slice conversion
go/internal/gccgoimporter/ar.go:95:83: zero-copy byte-slice conversion
index/suffixarray/suffixarray.go:294:15: zero-copy byte-slice conversion
internal/trace/parser.go:308:35: zero-copy byte-slice conversion
internal/trace/writer.go:16:16: zero-copy byte-slice conversion
net/dnsclient_unix.go:241:30: zero-copy byte-slice conversion
net/dnsclient_unix.go:241:30: zero-copy byte-slice conversion
net/http/client.go:243:53: zero-copy byte-slice conversion
net/http/client.go:419:49: zero-copy byte-slice conversion
net/http/fcgi/child.go:304:51: zero-copy byte-slice conversion
net/http/h2_bundle.go:4053:38: zero-copy byte-slice conversion
net/http/httputil/dump.go:163:35: zero-copy byte-slice conversion
net/http/request.go:975:50: zero-copy byte-slice conversion
net/http/transport.go:845:30: zero-copy byte-slice conversion
os/file.go:249:23: zero-copy byte-slice conversion
reflect/type.go:2370:30: zero-copy byte-slice conversion
reflect/type.go:2411:27: zero-copy byte-slice conversion
reflect/type.go:2560:28: zero-copy byte-slice conversion
regexp/regexp.go:712:26: zero-copy byte-slice conversion
strconv/atoi.go:120:26: zero-copy byte-slice conversion
strconv/atoi.go:240:28: zero-copy byte-slice conversion
syscall/exec_linux.go:169:16: zero-copy byte-slice conversion
syscall/exec_linux.go:174:22: zero-copy byte-slice conversion
syscall/exec_linux.go:175:16: zero-copy byte-slice conversion
syscall/exec_linux.go:178:22: zero-copy byte-slice conversion
syscall/exec_linux.go:180:22: zero-copy byte-slice conversion
syscall/exec_linux.go:576:29: zero-copy byte-slice conversion
syscall/exec_linux.go:613:16: zero-copy byte-slice conversion
syscall/exec_linux.go:615:16: zero-copy byte-slice conversion
syscall/lsf_linux.go:57:26: zero-copy byte-slice conversion
syscall/syscall_unix.go:333:29: zero-copy byte-slice conversion
vendor/golang.org/x/crypto/cryptobyte/asn1.go:116:20: zero-copy byte-slice conversion
vendor/golang.org/x/crypto/cryptobyte/asn1.go:116:20: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1847:15: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1847:15: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1851:24: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1851:24: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1857:19: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1857:19: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1857:19: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:1857:19: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:2379:31: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:2379:31: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:2381:35: zero-copy byte-slice conversion
vendor/golang.org/x/net/dns/dnsmessage/message.go:2381:35: zero-copy byte-slice conversion
Comment From: mdempsky
Hm, CL 285681 tries to zero-copy this conversion, which would be bad:
package p
import "syscall"
func f() {
syscall.Read(0, []byte("bad"))
}
I thought I was already conservatively handling when pointers are passed as uintptr to assembly functions, but apparently not. I'll have to dig into what's going on there.
Though it also means we'll probably need a way to distinguish syscalls that may mutate memory (e.g., Read) vs syscalls that won't (e.g., Write).
Looking through the 188 instances, I notice there are also some cases of range []byte(s)
, which we already recognize and optimize. Though it's nice to see this optimization generalizes it.
Comment From: methane
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.
}
Comment From: randall77
As my understanding, []byte -> string conversion don't require read-only checks. It require only escape analysis that current Go has. Am I correct?
No, at least if I understand you correctly. We need to handle this case:
b := []byte{'x'}
s := string(b)
b[0] = 'y'
The string s
should remain "x"
after the last assignment.
So it isn't that b
is read-only always, but that it is read-only after the conversion to string.
And "after" is hard, as b
may not be the only []byte
pointing at its backing array. Some sort of alias analysis is required. (Of course in code I showed nothing else aliases the backing store, but that may not be the case in more realistic scenarios.)
In any case, this issue is for the conversion the other way, string -> []byte.
Comment From: methane
Thank you.
I thought about string() in argument list like data := encodeString(string([]data))
.
But after reading your response, I noticed even encodeString(string([]data))
can have aliasing via global variable. That is why restrict
keyword is added for C99.
Now I understand why it is difficult than just escape analysis.
In any case, this issue is for the conversion the other way, string -> []byte.
Oh, I didn't know that. Would you change the issue title from string <-> []byte
to string -> []byte
?
Comment From: mattn
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
Comment From: jfcg
In any case, this issue is for the conversion the other way, string -> []byte.
Simpler example (tested with Go 1.17.8 / 1.18): pkg.go
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")
}
}
go test -v -bench ToLower -benchmem -count=3
prints
BenchmarkToLower-4 18710726 66.86 ns/op 32 B/op 2 allocs/op
BenchmarkToLower-4 17420910 65.57 ns/op 32 B/op 2 allocs/op
BenchmarkToLower-4 17085405 63.16 ns/op 32 B/op 2 allocs/op
There is a redundant allocation & copy in return string(buf)
. It looks too trivial to me that I am surprised Go compiler cannot handle this.
Comment From: gopherbot
Change https://go.dev/cl/520259 mentions this issue: cmd/compile/internal/escape: optimize indirect closure calls
Comment From: gopherbot
Change https://go.dev/cl/520395 mentions this issue: cmd/compile: enable zero-copy string->[]byte conversions
Comment From: dmitshur
CL 520395 that resolved this issue was reverted in CL 520596, so reopening.
Comment From: gopherbot
Change https://go.dev/cl/520599 mentions this issue: cmd/compile: restore zero-copy string->[]byte optimization
Comment From: gopherbot
Change https://go.dev/cl/520600 mentions this issue: cmd/compile: enable -d=zerocopy by default
Comment From: jfcg
Hi,
These changes address redundant copying for string <--> []byte
conversions both ways, right?
Comment From: mdempsky
@jfcg No, at this time only string->[]byte.
Comment From: cespare
I believe that #43752 discusses at least some of the []byte->string
cases.
(@mdempsky it's exciting that this 4-digit issue is so close to being solved!)
Comment From: zigo101
BTW, should the old optimization for for range []byte(aString)
be unified into the new optimization? Maybe it has been?
Comment From: gopherbot
Change https://go.dev/cl/584118 mentions this issue: encoding/hex: don't overallocate memory in DecodeString
Comment From: zigo101
So now we can take string element addresses without unsafe.StringData
:
package main
func main() {
s := "go"
// All true.
println(StringData(s) == StringData(s))
println(*StringData(s) == 'g')
println(*StringData(s[1:]) == 'o')
}
func StringData(s string) *byte {
if len(s) == 0 {
return nil
}
return &[]byte(s)[0]
}
;D