cmd/gc: don't copy []byte during string concatenation

Consider the following code:

s := "(" + string(byteSlice) + ")"

Currently we allocate a new string during []byte->string conversion,
and pass it to concatstrings. String allocation is unnecessary in
this case, because concatstrings does memorize the strings for later use.
This change uses slicebytetostringtmp to construct temp string directly
from []byte buffer and passes it to concatstrings.

I've found few such cases in std lib:

	s += string(msg[off:off+c]) + "."
	buf.WriteString("Sec-WebSocket-Accept: " + string(c.accept) + "\r\n")
	bw.WriteString("Sec-WebSocket-Key: " + string(nonce) + "\r\n")
	err = xml.Unmarshal([]byte("<Top>"+string(data)+"</Top>"), &logStruct)
	d.err = d.syntaxError("invalid XML name: " + string(b))
	return m, ProtocolError("malformed MIME header line: " + string(kv))

But there are much more in our internal code base.

Change-Id: I42f401f317131237ddd0cb9786b0940213af16fb
Reviewed-on: https://go-review.googlesource.com/3163
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/gc/order.c b/src/cmd/gc/order.c
index a1aa1bd300cc33fcd99add5de23a4c300d240ffd..2cd155d4d7391550adae6afd109093ac95c5a5de 100644 (file)
--- a/src/cmd/gc/order.c
+++ b/src/cmd/gc/order.c
@@ -58,6 +58,13 @@ static void  orderexprlistinplace(NodeList*, Order*);
 void
 order(Node *fn)
 {
+       char s[50];
+
+       if(debug['W'] > 1) {
+               snprint(s, sizeof(s), "\nbefore order %S", fn->nname->sym);
+               dumplist(s, fn->nbody);
+       }
+
        orderblock(&fn->nbody);
 }
 
@@ -974,7 +981,7 @@ orderexpr(Node **np, Order *order)
        Node *n;
        NodeList *mark, *l;
        Type *t;
-       int lno;
+       int lno, haslit, hasbyte;
 
        n = *np;
        if(n == N)
@@ -1002,6 +1009,26 @@ orderexpr(Node **np, Order *order)
                        t->type = types[TSTRING];
                        n->alloc = ordertemp(t, order, 0);
                }
+
+               // Mark string(byteSlice) arguments to reuse byteSlice backing
+               // buffer during conversion. String concatenation does not
+               // memorize the strings for later use, so it is safe.
+               // However, we can do it only if there is at least one non-empty string literal.
+               // Otherwise if all other arguments are empty strings,
+               // concatstrings will return the reference to the temp string
+               // to the caller.
+               hasbyte = 0;
+               haslit = 0;
+               for(l=n->list; l != nil; l=l->next) {
+                       hasbyte |= l->n->op == OARRAYBYTESTR;
+                       haslit |= l->n->op == OLITERAL && l->n->val.u.sval->len != 0;
+               }
+               if(haslit && hasbyte) {
+                       for(l=n->list; l != nil; l=l->next) {
+                               if(l->n->op == OARRAYBYTESTR)
+                                       l->n->op = OARRAYBYTESTRTMP;
+                       }
+               }
                break;
 
        case OINDEXMAP:

diff --git a/src/runtime/malloc_test.go b/src/runtime/malloc_test.go
index b7795aa1d66071049fdd88d7a89c7b67d04d462f..454b3c36fe816f6dcc47af0689e47ccfb3c98c5f 100644 (file)
--- a/src/runtime/malloc_test.go
+++ b/src/runtime/malloc_test.go
@@ -46,6 +46,23 @@ func TestMemStats(t *testing.T) {
        }
 }
 
+func TestStringConcatenationAllocs(t *testing.T) {
+       n := testing.AllocsPerRun(1e3, func() {
+               b := make([]byte, 10)
+               for i := 0; i < 10; i++ {
+                       b[i] = byte(i) + '0'
+               }
+               s := "foo" + string(b)
+               if want := "foo0123456789"; s != want {
+                       t.Fatalf("want %v, got %v", want, s)
+               }
+       })
+       // Only string concatenation allocates.
+       if n != 1 {
+               t.Fatalf("want 1 allocation, got %v", n)
+       }
+}
+
 var mallocSink uintptr
 
 func BenchmarkMalloc8(b *testing.B) {

diff --git a/src/runtime/string.go b/src/runtime/string.go
index 96f95796243acfd308b8df3bdb8da74f0b25c341..8aa0dd076d0ddf233eca152c82f47e3995f02339 100644 (file)
--- a/src/runtime/string.go
+++ b/src/runtime/string.go
@@ -73,8 +73,9 @@ func slicebytetostringtmp(b []byte) string {
        // that know that the string form will be discarded before
        // the calling goroutine could possibly modify the original
        // slice or synchronize with another goroutine.
-       // Today, the only such case is a m[string(k)] lookup where
+       // First such case is a m[string(k)] lookup where
        // m is a string-keyed map and k is a []byte.
+       // Second such case is "<"+string(b)+">" concatenation where b is []byte.
 
        if raceenabled && len(b) > 0 {
                racereadrangepc(unsafe.Pointer(&b[0]),