]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/gc, runtime: optimize map[string] lookup from []byte key
authorRuss Cox <rsc@golang.org>
Thu, 3 Apr 2014 23:05:17 +0000 (19:05 -0400)
committerRuss Cox <rsc@golang.org>
Thu, 3 Apr 2014 23:05:17 +0000 (19:05 -0400)
Brad has been asking for this for a while.
I have resisted because I wanted to find a more general way to
do this, one that would keep the performance of code introducing
variables the same as the performance of code that did not.
(See golang.org/issue/3512#c20).

I have not found the more general way, and recent changes to
remove ambiguously live temporaries have blown away the
property I was trying to preserve, so that's no longer a reason
not to make the change.

Fixes #3512.

LGTM=iant
R=iant
CC=bradfitz, golang-codereviews, khr, r
https://golang.org/cl/83740044

src/cmd/gc/builtin.c
src/cmd/gc/go.h
src/cmd/gc/order.c
src/cmd/gc/runtime.go
src/cmd/gc/walk.c
src/pkg/runtime/map_test.go
src/pkg/runtime/string.goc

index 1f4aed5baa404e856ac0c76f62a3f0d24f6896e4..5ca5aeb770057b1a30e7323ee67d937cda029683 100644 (file)
@@ -33,6 +33,7 @@ char *runtimeimport =
        "func @\"\".eqstring (? string, ? string) (? bool)\n"
        "func @\"\".intstring (? int64) (? string)\n"
        "func @\"\".slicebytetostring (? []byte) (? string)\n"
+       "func @\"\".slicebytetostringtmp (? []byte) (? string)\n"
        "func @\"\".slicerunetostring (? []rune) (? string)\n"
        "func @\"\".stringtoslicebyte (? string) (? []byte)\n"
        "func @\"\".stringtoslicerune (? string) (? []rune)\n"
index 01dfe7fed7446dd740890e40ac70eec404cd6c26..125ae9cf4401247e9a64ef7607536ef720c069f2 100644 (file)
@@ -450,6 +450,7 @@ enum
        OANDAND,        // b0 && b1
        OAPPEND,        // append
        OARRAYBYTESTR,  // string(bytes)
+       OARRAYBYTESTRTMP, // string(bytes) ephemeral
        OARRAYRUNESTR,  // string(runes)
        OSTRARRAYBYTE,  // []byte(s)
        OSTRARRAYRUNE,  // []rune(s)
index 5fec73854d47862f5917cede3dfeb8ab491bd059..29eb242b10d610c1bdc36939ec2e0781b008e8d5 100644 (file)
@@ -565,9 +565,13 @@ orderstmt(Node *n, Order *order)
                // and make sure OINDEXMAP is not copied out.
                t = marktemp(order);
                orderexprlist(n->list, order);
-               orderexpr(&n->rlist->n->left, order);
-               orderexpr(&n->rlist->n->right, order);
-               orderaddrtemp(&n->rlist->n->right, order);
+               r = n->rlist->n;
+               orderexpr(&r->left, order);
+               orderexpr(&r->right, order);
+               // See case OINDEXMAP below.
+               if(r->right->op == OARRAYBYTESTR)
+                       r->right->op = OARRAYBYTESTRTMP;
+               orderaddrtemp(&r->right, order);
                ordermapassign(n, order);
                cleantemp(t, order);
                break;
@@ -935,6 +939,20 @@ orderexpr(Node **np, Order *order)
                // key must be addressable
                orderexpr(&n->left, order);
                orderexpr(&n->right, order);
+
+               // For x = m[string(k)] where k is []byte, the allocation of
+               // backing bytes for the string can be avoided by reusing
+               // the []byte backing array. This is a special case that it
+               // would be nice to handle more generally, but because
+               // there are no []byte-keyed maps, this specific case comes
+               // up in important cases in practice. See issue 3512.
+               // Nothing can change the []byte we are not copying before
+               // the map index, because the map access is going to
+               // be forced to happen immediately following this
+               // conversion (by the ordercopyexpr a few lines below).
+               if(n->etype == 0 && n->right->op == OARRAYBYTESTR)
+                       n->right->op = OARRAYBYTESTRTMP;
+
                orderaddrtemp(&n->right, order);
                if(n->etype == 0) {
                        // use of value (not being assigned);
index 9ea4a79fd3bbe11b906210d5d79c270c8ebec13b..fb5c2a150ec4850eff14ffd548a465904ca7b0ca 100644 (file)
@@ -47,6 +47,7 @@ func cmpstring(string, string) int
 func eqstring(string, string) bool
 func intstring(int64) string
 func slicebytetostring([]byte) string
+func slicebytetostringtmp([]byte) string
 func slicerunetostring([]rune) string
 func stringtoslicebyte(string) []byte
 func stringtoslicerune(string) []rune
index 1ffe8937f80530afa8ac9e2bc82d29f3f6c5ff83..3bb48fdbbf3c319298c80c00f64da65a7681ed8e 100644 (file)
@@ -1316,6 +1316,11 @@ walkexpr(Node **np, NodeList **init)
                n = mkcall("slicebytetostring", n->type, init, n->left);
                goto ret;
 
+       case OARRAYBYTESTRTMP:
+               // slicebytetostringtmp([]byte) string;
+               n = mkcall("slicebytetostringtmp", n->type, init, n->left);
+               goto ret;
+
        case OARRAYRUNESTR:
                // slicerunetostring([]rune) string;
                n = mkcall("slicerunetostring", n->type, init, n->left);
index 9c703ba362e99eab738a668102175467e9ed350f..e4e8383493623fe9a27e0fe619ed188c8a390db9 100644 (file)
@@ -438,3 +438,40 @@ func TestMapIterOrder(t *testing.T) {
                }
        }
 }
+
+func TestMapStringBytesLookup(t *testing.T) {
+       // Use large string keys to avoid small-allocation coalescing,
+       // which can cause AllocsPerRun to report lower counts than it should.
+       m := map[string]int{
+               "1000000000000000000000000000000000000000000000000": 1,
+               "2000000000000000000000000000000000000000000000000": 2,
+       }
+       buf := []byte("1000000000000000000000000000000000000000000000000")
+       if x := m[string(buf)]; x != 1 {
+               t.Errorf(`m[string([]byte("1"))] = %d, want 1`, x)
+       }
+       buf[0] = '2'
+       if x := m[string(buf)]; x != 2 {
+               t.Errorf(`m[string([]byte("2"))] = %d, want 2`, x)
+       }
+
+       var x int
+       n := testing.AllocsPerRun(100, func() {
+               x += m[string(buf)]
+       })
+       if n != 0 {
+               t.Errorf("AllocsPerRun for m[string(buf)] = %v, want 0", n)
+       }
+
+       x = 0
+       n = testing.AllocsPerRun(100, func() {
+               y, ok := m[string(buf)]
+               if !ok {
+                       panic("!ok")
+               }
+               x += y
+       })
+       if n != 0 {
+               t.Errorf("AllocsPerRun for x,ok = m[string(buf)] = %v, want 0", n)
+       }
+}
index 89b9130c089faa83220a16e620daf0c78271b99e..97a69d07b1bb73636a28c05302498cee067d6f01 100644 (file)
@@ -294,6 +294,25 @@ func slicebytetostring(b Slice) (s String) {
        runtime·memmove(s.str, b.array, s.len);
 }
 
+func slicebytetostringtmp(b Slice) (s String) {
+       void *pc;
+
+       if(raceenabled) {
+               pc = runtime·getcallerpc(&b);
+               runtime·racereadrangepc(b.array, b.len, pc, runtime·slicebytetostringtmp);
+       }
+       
+       // Return a "string" referring to the actual []byte bytes.
+       // This is only for use by internal compiler optimizations
+       // 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
+       // m is a string-keyed map and k is a []byte.
+       s.str = b.array;
+       s.len = b.len;
+}
+
 func stringtoslicebyte(s String) (b Slice) {
        uintptr cap;