runtime: Simplify slice growing/appending code (#4287)

* reflect: rawFieldByNameFunc: copy index slice to avoid later overwrites

* runtime: Simplify slice growing/appending code

Refactor the slice appending function to rely on the slice growing
function, and remove branches/loops to use a branchfree variant.

Signed-off-by: L. Pereira <l.pereira@fastly.com>

* runtime: Remove one branch in sliceAppend()

Both branches were equivalent, so guard the overall logic in
sliceAppend() with the more general condition.

Signed-off-by: L. Pereira <l.pereira@fastly.com>

* runtime: Simplify slice growing calculation

Use `bits.Len()` rather than `32 - bits.LeadingZeros32()`.  They're
equivalent, but the Len version is a bit easier to read.

Signed-off-by: L. Pereira <l.pereira@fastly.com>

* reflect: Always call sliceGrow() in extendSlice()

sliceGrow() will return the old slice if its capacity is large enough.

Signed-off-by: L. Pereira <l.pereira@fastly.com>

---------

Signed-off-by: L. Pereira <l.pereira@fastly.com>
Co-authored-by: Damian Gryski <damian@gryski.com>

src/runtime/slice.go

@@ -3,42 +3,24 @@ package runtime
 // This file implements compiler builtins for slices: append() and copy().
 
 import (
+	"math/bits"
 	"unsafe"
 )
 
 // Builtin append(src, elements...) function: append elements to src and return
 // the modified (possibly expanded) slice.
-func sliceAppend(srcBuf, elemsBuf unsafe.Pointer, srcLen, srcCap, elemsLen uintptr, elemSize uintptr) (unsafe.Pointer, uintptr, uintptr) {
-	if elemsLen == 0 {
-		// Nothing to append, return the input slice.
-		return srcBuf, srcLen, srcCap
+func sliceAppend(srcBuf, elemsBuf unsafe.Pointer, srcLen, srcCap, elemsLen, elemSize uintptr) (unsafe.Pointer, uintptr, uintptr) {
+	newLen := srcLen + elemsLen
+	if elemsLen > 0 {
+		// Allocate a new slice with capacity for elemsLen more elements, if necessary;
+		// otherwise, reuse the passed slice.
+		srcBuf, _, srcCap = sliceGrow(srcBuf, srcLen, srcCap, newLen, elemSize)
+
+		// Append the new elements in-place.
+		memmove(unsafe.Add(srcBuf, srcLen*elemSize), elemsBuf, elemsLen*elemSize)
 	}
 
-	if srcLen+elemsLen > srcCap {
-		// Slice does not fit, allocate a new buffer that's large enough.
-		srcCap = srcCap * 2
-		if srcCap == 0 { // e.g. zero slice
-			srcCap = 1
-		}
-		for srcLen+elemsLen > srcCap {
-			// This algorithm may be made more memory-efficient: don't multiply
-			// by two but by 1.5 or something. As far as I can see, that's
-			// allowed by the Go language specification (but may be observed by
-			// programs).
-			srcCap *= 2
-		}
-		buf := alloc(srcCap*elemSize, nil)
-
-		// Copy the old slice to the new slice.
-		if srcLen != 0 {
-			memmove(buf, srcBuf, srcLen*elemSize)
-		}
-		srcBuf = buf
-	}
-
-	// The slice fits (after possibly allocating a new one), append it in-place.
-	memmove(unsafe.Add(srcBuf, srcLen*elemSize), elemsBuf, elemsLen*elemSize)
-	return srcBuf, srcLen + elemsLen, srcCap
+	return srcBuf, newLen, srcCap
 }
 
 // Builtin copy(dst, src) function: copy bytes from dst to src.
@@ -54,29 +36,22 @@ func sliceCopy(dst, src unsafe.Pointer, dstLen, srcLen uintptr, elemSize uintptr
 
 // sliceGrow returns a new slice with space for at least newCap elements
 func sliceGrow(oldBuf unsafe.Pointer, oldLen, oldCap, newCap, elemSize uintptr) (unsafe.Pointer, uintptr, uintptr) {
-
-	// TODO(dgryski): sliceGrow() and sliceAppend() should be refactored to share the base growth code.
-
 	if oldCap >= newCap {
 		// No need to grow, return the input slice.
 		return oldBuf, oldLen, oldCap
 	}
 
-	// allow nil slice
-	if oldCap == 0 {
-		oldCap++
-	}
+	// This can be made more memory-efficient by multiplying by some other constant, such as 1.5,
+	// which seems to be allowed by the Go language specification (but this can be observed by
+	// programs); however, due to memory fragmentation and the current state of the TinyGo
+	// memory allocators, this causes some difficult to debug issues.
+	newCap = 1 << bits.Len(uint(newCap))
 
-	// grow capacity
-	for oldCap < newCap {
-		oldCap *= 2
-	}
-
-	buf := alloc(oldCap*elemSize, nil)
+	buf := alloc(newCap*elemSize, nil)
 	if oldLen > 0 {
 		// copy any data to new slice
 		memmove(buf, oldBuf, oldLen*elemSize)
 	}
 
-	return buf, oldLen, oldCap
+	return buf, oldLen, newCap
 }