From 790fa1c546a05936406f6bbf24f6a6ddeb6ec6ad Mon Sep 17 00:00:00 2001 From: Martin Möhrmann Date: Mon, 14 Sep 2020 16:30:43 +0200 Subject: cmd/compile: unify reflect, string and slice copy runtime functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a common runtime slicecopy function to copy strings or slices into slices. This deduplicates similar code previously used in reflect.slicecopy and runtime.stringslicecopy. Change-Id: I09572ff0647a9e12bb5c6989689ce1c43f16b7f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/254658 Run-TryBot: Martin Möhrmann TryBot-Result: Go Bot Trust: Martin Möhrmann Reviewed-by: Keith Randall --- src/runtime/slice.go | 44 +++++++++++--------------------------------- 1 file changed, 11 insertions(+), 33 deletions(-) (limited to 'src/runtime/slice.go') diff --git a/src/runtime/slice.go b/src/runtime/slice.go index 0418ace25a..82a45c78a9 100644 --- a/src/runtime/slice.go +++ b/src/runtime/slice.go @@ -243,12 +243,13 @@ func isPowerOfTwo(x uintptr) bool { return x&(x-1) == 0 } -func slicecopy(toPtr unsafe.Pointer, toLen int, fmPtr unsafe.Pointer, fmLen int, width uintptr) int { - if fmLen == 0 || toLen == 0 { +// slicecopy is used to copy from a string or slice of pointerless elements into a slice. +func slicecopy(toPtr unsafe.Pointer, toLen int, fromPtr unsafe.Pointer, fromLen int, width uintptr) int { + if fromLen == 0 || toLen == 0 { return 0 } - n := fmLen + n := fromLen if toLen < n { n = toLen } @@ -257,46 +258,23 @@ func slicecopy(toPtr unsafe.Pointer, toLen int, fmPtr unsafe.Pointer, fmLen int, return n } + size := uintptr(n) * width if raceenabled { callerpc := getcallerpc() pc := funcPC(slicecopy) - racereadrangepc(fmPtr, uintptr(n*int(width)), callerpc, pc) - racewriterangepc(toPtr, uintptr(n*int(width)), callerpc, pc) + racereadrangepc(fromPtr, size, callerpc, pc) + racewriterangepc(toPtr, size, callerpc, pc) } if msanenabled { - msanread(fmPtr, uintptr(n*int(width))) - msanwrite(toPtr, uintptr(n*int(width))) + msanread(fromPtr, size) + msanwrite(toPtr, size) } - size := uintptr(n) * width if size == 1 { // common case worth about 2x to do here // TODO: is this still worth it with new memmove impl? - *(*byte)(toPtr) = *(*byte)(fmPtr) // known to be a byte pointer + *(*byte)(toPtr) = *(*byte)(fromPtr) // known to be a byte pointer } else { - memmove(toPtr, fmPtr, size) - } - return n -} - -func slicestringcopy(toPtr *byte, toLen int, fm string) int { - if len(fm) == 0 || toLen == 0 { - return 0 - } - - n := len(fm) - if toLen < n { - n = toLen + memmove(toPtr, fromPtr, size) } - - if raceenabled { - callerpc := getcallerpc() - pc := funcPC(slicestringcopy) - racewriterangepc(unsafe.Pointer(toPtr), uintptr(n), callerpc, pc) - } - if msanenabled { - msanwrite(unsafe.Pointer(toPtr), uintptr(n)) - } - - memmove(unsafe.Pointer(toPtr), stringStructOf(&fm).str, uintptr(n)) return n } -- cgit v1.3 From 2333c6299f340a5f76a73a4fec6db23ffa388e97 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 24 Sep 2020 19:26:33 -0700 Subject: runtime: use old capacity to decide on append growth regime We grow the backing store on append by 2x for small sizes and 1.25x for large sizes. The threshold we use for picking the growth factor used to depend on the old length, not the old capacity. That's kind of unfortunate, because then doing append(s, 0, 0) and append(append(s, 0), 0) do different things. (If s has one more spot available, then the former expression chooses its growth based on len(s) and the latter on len(s)+1.) If we instead use the old capacity, we get more consistent behavior. (Both expressions use len(s)+1 == cap(s) to decide.) Fixes #41239 Change-Id: I40686471d256edd72ec92aef973a89b52e235d4b Reviewed-on: https://go-review.googlesource.com/c/go/+/257338 Trust: Keith Randall Trust: Josh Bleecher Snyder Run-TryBot: Keith Randall TryBot-Result: Go Bot Reviewed-by: Josh Bleecher Snyder --- src/runtime/slice.go | 2 +- test/fixedbugs/issue41239.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue41239.go (limited to 'src/runtime/slice.go') diff --git a/src/runtime/slice.go b/src/runtime/slice.go index 82a45c78a9..c0647d95a0 100644 --- a/src/runtime/slice.go +++ b/src/runtime/slice.go @@ -146,7 +146,7 @@ func growslice(et *_type, old slice, cap int) slice { if cap > doublecap { newcap = cap } else { - if old.len < 1024 { + if old.cap < 1024 { newcap = doublecap } else { // Check 0 < newcap to detect overflow diff --git a/test/fixedbugs/issue41239.go b/test/fixedbugs/issue41239.go new file mode 100644 index 0000000000..3e9ef5eb66 --- /dev/null +++ b/test/fixedbugs/issue41239.go @@ -0,0 +1,19 @@ +// run + +// Copyright 2020 The Go Authors. All rights reserved. Use of this +// source code is governed by a BSD-style license that can be found in +// the LICENSE file. + +package main + +import "fmt" + +func main() { + const N = 1024 + var a [N]int + x := cap(append(a[:N-1:N], 9, 9)) + y := cap(append(a[:N:N], 9)) + if x != y { + panic(fmt.Sprintf("different capacity on append: %d vs %d", x, y)) + } +} -- cgit v1.3