runtime: reduce slice growth during append to 2x

The new inlined code for append assumed that it could pass the
desired new cap to growslice, not the number of new elements.
But growslice still interpreted the argument as the number of new elements,
making it always grow by >2x (more precisely, 2x+1 rounded up
to the next malloc block size). At the time, I had intended to change
the other callers to use the new cap as well, but it's too late for that.
Instead, introduce growslice_n for the old callers and keep growslice
for the inlined (common case) caller.

Fixes #11403.

Filed #11419 to merge them.

Change-Id: I1338b1e5b352f3be4e43641f44b652ef7195251b
Reviewed-on: https://go-review.googlesource.com/11541
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/cmd/compile/internal/gc/builtin.go b/src/cmd/compile/internal/gc/builtin.go
index 6bdf78c8abd73dea42509fbacb0a3b55c189dd9a..f09dd5690f9b327f1a7801b5b3f5368560cc7b94 100644 (file)
--- a/src/cmd/compile/internal/gc/builtin.go
+++ b/src/cmd/compile/internal/gc/builtin.go
@@ -130,7 +130,8 @@ const runtimeimport = "" +
        "func @\"\".selectgo (@\"\".sel·1 *byte)\n" +
        "func @\"\".block ()\n" +
        "func @\"\".makeslice (@\"\".typ·2 *byte, @\"\".nel·3 int64, @\"\".cap·4 int64) (@\"\".ary·1 []any)\n" +
-       "func @\"\".growslice (@\"\".typ·2 *byte, @\"\".old·3 []any, @\"\".n·4 int) (@\"\".ary·1 []any)\n" +
+       "func @\"\".growslice (@\"\".typ·2 *byte, @\"\".old·3 []any, @\"\".cap·4 int) (@\"\".ary·1 []any)\n" +
+       "func @\"\".growslice_n (@\"\".typ·2 *byte, @\"\".old·3 []any, @\"\".n·4 int) (@\"\".ary·1 []any)\n" +
        "func @\"\".memmove (@\"\".to·1 *any, @\"\".frm·2 *any, @\"\".length·3 uintptr)\n" +
        "func @\"\".memclr (@\"\".ptr·1 *byte, @\"\".length·2 uintptr)\n" +
        "func @\"\".memequal (@\"\".x·2 *any, @\"\".y·3 *any, @\"\".size·4 uintptr) (? bool)\n" +

diff --git a/src/cmd/compile/internal/gc/builtin/runtime.go b/src/cmd/compile/internal/gc/builtin/runtime.go
index 179a4ddd9aba4da1e1639c350152696dddfadb90..6210f10cdf457e45b34f2bf1a907cd6cb8e96fd5 100644 (file)
--- a/src/cmd/compile/internal/gc/builtin/runtime.go
+++ b/src/cmd/compile/internal/gc/builtin/runtime.go
@@ -162,7 +162,8 @@ func selectgo(sel *byte)
 func block()
 
 func makeslice(typ *byte, nel int64, cap int64) (ary []any)
-func growslice(typ *byte, old []any, n int) (ary []any)
+func growslice(typ *byte, old []any, cap int) (ary []any)
+func growslice_n(typ *byte, old []any, n int) (ary []any)
 func memmove(to *any, frm *any, length uintptr)
 func memclr(ptr *byte, length uintptr)
 

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index bbbc990cb17806ad130b776019422db175fe22ba..f5ae9fbe21559ba140431b4bd3fb37de68f099a5 100644 (file)
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -2884,7 +2884,7 @@ func addstr(n *Node, init **NodeList) *Node {
 //   init {
 //     s := l1
 //     if n := len(l1) + len(l2) - cap(s); n > 0 {
-//       s = growslice(s, n)
+//       s = growslice_n(s, n)
 //     }
 //     s = s[:len(l1)+len(l2)]
 //     memmove(&s[len(l1)], &l2[0], len(l2)*sizeof(T))
@@ -2918,11 +2918,11 @@ func appendslice(n *Node, init **NodeList) *Node {
 
        nif.Left = Nod(OGT, nt, Nodintconst(0))
 
-       // instantiate growslice(Type*, []any, int) []any
-       fn := syslook("growslice", 1) //   growslice(<type>, old []T, n int64) (ret []T)
+       // instantiate growslice_n(Type*, []any, int) []any
+       fn := syslook("growslice_n", 1) //   growslice_n(<type>, old []T, n int64) (ret []T)
        substArgTypes(fn, s.Type.Type, s.Type.Type)
 
-       // s = growslice(T, s, n)
+       // s = growslice_n(T, s, n)
        nif.Nbody = list1(Nod(OAS, s, mkcall1(fn, s.Type, &nif.Ninit, typename(s.Type), s, nt)))
 
        l = list(l, nif)
@@ -2997,7 +2997,7 @@ func appendslice(n *Node, init **NodeList) *Node {
 //     s := src
 //     const argc = len(args) - 1
 //     if cap(s) - len(s) < argc {
-//         s = growslice(s, argc)
+//         s = growslice(s, len(s)+argc)
 //     }
 //     n := len(s)
 //     s = s[:n+argc]
@@ -3050,10 +3050,10 @@ func walkappend(n *Node, init **NodeList, dst *Node) *Node {
        nx := Nod(OIF, nil, nil)       // if cap(s) - len(s) < argc
        nx.Left = Nod(OLT, Nod(OSUB, Nod(OCAP, ns, nil), Nod(OLEN, ns, nil)), na)
 
-       fn := syslook("growslice", 1) //   growslice(<type>, old []T, n int) (ret []T)
+       fn := syslook("growslice", 1) //   growslice(<type>, old []T, mincap int) (ret []T)
        substArgTypes(fn, ns.Type.Type, ns.Type.Type)
 
-       nx.Nbody = list1(Nod(OAS, ns, mkcall1(fn, ns.Type, &nx.Ninit, typename(ns.Type), ns, na)))
+       nx.Nbody = list1(Nod(OAS, ns, mkcall1(fn, ns.Type, &nx.Ninit, typename(ns.Type), ns, Nod(OADD, Nod(OLEN, ns, nil), na))))
 
        l = list(l, nx)
 

diff --git a/src/runtime/runtime_test.go b/src/runtime/runtime_test.go
index f65562ab9182600da83228613f5658f8956ef5fd..75fc9bcb844150c47695fa263e094250f0d38a99 100644 (file)
--- a/src/runtime/runtime_test.go
+++ b/src/runtime/runtime_test.go
@@ -261,3 +261,43 @@ func TestBadOpen(t *testing.T) {
                t.Errorf("close()=%d, want -1", c)
        }
 }
+
+func TestAppendGrowth(t *testing.T) {
+       var x []int64
+       check := func(want int) {
+               if cap(x) != want {
+                       t.Errorf("len=%d, cap=%d, want cap=%d", len(x), cap(x), want)
+               }
+       }
+
+       check(0)
+       want := 1
+       for i := 1; i <= 100; i++ {
+               x = append(x, 1)
+               check(want)
+               if i&(i-1) == 0 {
+                       want = 2 * i
+               }
+       }
+}
+
+var One = []int64{1}
+
+func TestAppendSliceGrowth(t *testing.T) {
+       var x []int64
+       check := func(want int) {
+               if cap(x) != want {
+                       t.Errorf("len=%d, cap=%d, want cap=%d", len(x), cap(x), want)
+               }
+       }
+
+       check(0)
+       want := 1
+       for i := 1; i <= 100; i++ {
+               x = append(x, One...)
+               check(want)
+               if i&(i-1) == 0 {
+                       want = 2 * i
+               }
+       }
+}

diff --git a/src/runtime/slice.go b/src/runtime/slice.go
index 15820a51811ea2083c8338ed68cf77f9518c971b..5cda11d9b0d0947822b73de2d7787053e08d3e71 100644 (file)
--- a/src/runtime/slice.go
+++ b/src/runtime/slice.go
@@ -33,12 +33,22 @@ func makeslice(t *slicetype, len64, cap64 int64) slice {
        return slice{p, len, cap}
 }
 
-func growslice(t *slicetype, old slice, n int) slice {
+// growslice_n is a variant of growslice that takes the number of new elements
+// instead of the new minimum capacity.
+// TODO(rsc): This is used by append(slice, slice...).
+// The compiler should change that code to use growslice directly (issue #11419).
+func growslice_n(t *slicetype, old slice, n int) slice {
        if n < 1 {
                panic(errorString("growslice: invalid n"))
        }
+       return growslice(t, old, old.cap+n)
+}
 
-       cap := old.cap + n
+// growslice handles slice growth during append.
+// It is passed the slice type, the old slice, and the desired new minimum capacity,
+// and it returns a new slice with at least that capacity, with the old data
+// copied into it.
+func growslice(t *slicetype, old slice, cap int) slice {
        if cap < old.cap || t.elem.size > 0 && uintptr(cap) > _MaxMem/uintptr(t.elem.size) {
                panic(errorString("growslice: cap out of range"))
        }