From: Russ Cox Date: Thu, 25 Jun 2015 23:27:20 +0000 (-0400) Subject: runtime: reduce slice growth during append to 2x X-Git-Tag: go1.5beta1~98 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=32fddadd98f938018485fba6253d30273db4e5e9;p=gostls13.git 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 --- diff --git a/src/cmd/compile/internal/gc/builtin.go b/src/cmd/compile/internal/gc/builtin.go index 6bdf78c8ab..f09dd5690f 100644 --- 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 179a4ddd9a..6210f10cdf 100644 --- 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 bbbc990cb1..f5ae9fbe21 100644 --- 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(, old []T, n int64) (ret []T) + // instantiate growslice_n(Type*, []any, int) []any + fn := syslook("growslice_n", 1) // growslice_n(, 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(, old []T, n int) (ret []T) + fn := syslook("growslice", 1) // growslice(, 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 f65562ab91..75fc9bcb84 100644 --- 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 15820a5181..5cda11d9b0 100644 --- 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")) }