runtime (gc_blocks.go): simplify scanning logic

Loop over valid pointer locations in heap objects instead of checking if each location is valid.
The conservative scanning code is now shared between markRoots and the heap scan.

This also removes the ending alignment requirement from markRoots, since the new scan* functions do not require an aligned length.
This requirement was occasionally violated by the linux global marking code.

This saves some code space and has negligible impact on performance.

builder/sizes_test.go

@@ -42,9 +42,9 @@ func TestBinarySize(t *testing.T) {
 	// This is a small number of very diverse targets that we want to test.
 	tests := []sizeTest{
 		// microcontrollers
-		{"hifive1b", "examples/echo", 3756, 280, 0, 2268},
-		{"microbit", "examples/serial", 2756, 340, 8, 2272},
-		{"wioterminal", "examples/pininterrupt", 7297, 1491, 116, 6912},
+		{"hifive1b", "examples/echo", 3568, 280, 0, 2268},
+		{"microbit", "examples/serial", 2630, 342, 8, 2272},
+		{"wioterminal", "examples/pininterrupt", 7175, 1493, 116, 6912},
 
 		// TODO: also check wasm. Right now this is difficult, because
 		// wasm binaries are run through wasm-opt and therefore the

src/runtime/gc_blocks.go

@@ -532,8 +532,7 @@ func runGC() (freeBytes uintptr) {
 
 // markRoots reads all pointers from start to end (exclusive) and if they look
 // like a heap pointer and are unmarked, marks them and scans that object as
-// well (recursively). The start and end parameters must be valid pointers and
-// must be aligned.
+// well (recursively). The starting address must be valid and aligned.
 func markRoots(start, end uintptr) {
 	if gcDebug {
 		println("mark from", start, "to", end, int(end-start))
@@ -545,18 +544,21 @@ func markRoots(start, end uintptr) {
 		if start%unsafe.Alignof(start) != 0 {
 			runtimePanic("gc: unaligned start pointer")
 		}
-		if end%unsafe.Alignof(end) != 0 {
-			runtimePanic("gc: unaligned end pointer")
-		}
 	}
 
-	// Reduce the end bound to avoid reading too far on platforms where pointer alignment is smaller than pointer size.
-	// If the size of the range is 0, then end will be slightly below start after this.
-	end -= unsafe.Sizeof(end) - unsafe.Alignof(end)
+	// Scan the range conservatively.
+	scanConservative(start, end-start)
+}
 
-	for addr := start; addr < end; addr += unsafe.Alignof(addr) {
+// scanConservative scans all possible pointer locations in a range and marks referenced heap allocations.
+// The starting address must be valid and pointer-aligned.
+func scanConservative(addr, len uintptr) {
+	for len >= unsafe.Sizeof(addr) {
 		root := *(*uintptr)(unsafe.Pointer(addr))
 		markRoot(addr, root)
+
+		addr += unsafe.Alignof(addr)
+		len -= unsafe.Alignof(addr)
 	}
 }
 
@@ -576,58 +578,21 @@ func finishMark() {
 		}
 		scanList = obj.next
 
-		// Create a scanner with the object layout.
-		scanner := obj.layout.scanner()
-		if scanner.pointerFree() {
+		// Check if the object may contain pointers.
+		if obj.layout.pointerFree() {
 			// This object doesn't contain any pointers.
 			// This is a fast path for objects like make([]int, 4096).
+			// It skips the length calculation.
 			continue
 		}
 
-		// Scan all pointers in the object.
-		start := uintptr(unsafe.Pointer(obj)) + align(unsafe.Sizeof(objHeader{}))
-		end := blockFromAddr(uintptr(unsafe.Pointer(obj))).findNext().address()
+		// Compute the scan bounds.
+		objAddr := uintptr(unsafe.Pointer(obj))
+		start := objAddr + align(unsafe.Sizeof(objHeader{}))
+		end := blockFromAddr(objAddr).findNext().address()
 
-		for addr := start; addr != end; addr += unsafe.Alignof(addr) {
-			// Load the word.
-			word := *(*uintptr)(unsafe.Pointer(addr))
-
-			if !scanner.nextIsPointer(word, uintptr(unsafe.Pointer(obj)), addr) {
-				// Not a heap pointer.
-				continue
-			}
-
-			// Find the corresponding memory block.
-			referencedBlock := blockFromAddr(word)
-
-			if referencedBlock.state() == blockStateFree {
-				// The to-be-marked object doesn't actually exist.
-				// This is probably a false positive.
-				if gcDebug {
-					println("found reference to free memory:", word, "at:", addr)
-				}
-				continue
-			}
-
-			// Move to the block's head.
-			referencedBlock = referencedBlock.findHead()
-
-			if referencedBlock.state() == blockStateMark {
-				// The block has already been marked by something else.
-				continue
-			}
-
-			// Mark block.
-			if gcDebug {
-				println("marking block:", referencedBlock)
-			}
-			referencedBlock.setState(blockStateMark)
-
-			// Add the object to the scan list.
-			header := (*objHeader)(referencedBlock.pointer())
-			header.next = scanList
-			scanList = header
-		}
+		// Scan the object.
+		obj.layout.scan(start, end-start)
 	}
 }
 

src/runtime/gc_conservative.go

@@ -8,34 +8,23 @@ package runtime
 
 import "unsafe"
 
-// gcLayout tracks pointer locations in a heap object.
-// The conservative GC treats all locations as potential pointers, so this doesn't need to store anything.
-type gcLayout struct {
-}
-
 // parseGCLayout stores the layout information passed to alloc into a gcLayout value.
 // The conservative GC discards this information.
 func parseGCLayout(layout unsafe.Pointer) gcLayout {
 	return gcLayout{}
 }
 
-// scanner creates a gcObjectScanner with this layout.
-func (l gcLayout) scanner() gcObjectScanner {
-	return gcObjectScanner{}
+// gcLayout tracks pointer locations in a heap object.
+// The conservative GC treats all locations as potential pointers, so this doesn't need to store anything.
+type gcLayout struct {
 }
 
-type gcObjectScanner struct {
-}
-
-func (scanner *gcObjectScanner) pointerFree() bool {
+func (l gcLayout) pointerFree() bool {
 	// We don't know whether this object contains pointers, so conservatively
 	// return false.
 	return false
 }
 
-// nextIsPointer returns whether this could be a pointer. Because the GC is
-// conservative, we can't do much more than check whether the object lies
-// somewhere in the heap.
-func (scanner gcObjectScanner) nextIsPointer(ptr, parent, addrOfWord uintptr) bool {
-	return isOnHeap(ptr)
+func (l gcLayout) scan(start, len uintptr) {
+	scanConservative(start, len)
 }

src/runtime/gc_precise.go

@@ -57,98 +57,96 @@ package runtime
 
 import "unsafe"
 
-const preciseHeap = true
+const sizeFieldBits = 4 + (unsafe.Sizeof(uintptr(0)) / 4)
 
 // parseGCLayout stores the layout information passed to alloc into a gcLayout value.
 func parseGCLayout(layout unsafe.Pointer) gcLayout {
-	return gcLayout{layout: uintptr(layout)}
+	return gcLayout(layout)
 }
 
 // gcLayout tracks pointer locations in a heap object.
-type gcLayout struct {
-	layout uintptr
+type gcLayout uintptr
+
+func (layout gcLayout) pointerFree() bool {
+	return layout&1 != 0 && layout>>(sizeFieldBits+1) == 0
 }
 
-// scanner creates a gcObjectScanner with this layout.
-func (l gcLayout) scanner() (scanner gcObjectScanner) {
-	layout := l.layout
-	if layout == 0 {
-		// Unknown layout. Assume all words in the object could be pointers.
-		// This layout value below corresponds to a slice of pointers like:
-		//     make(*byte, N)
-		scanner.size = 1
-		scanner.bitmap = 1
-	} else if layout&1 != 0 {
-		// Layout is stored directly in the integer value.
-		// Determine format of bitfields in the integer.
-		const layoutBits = uint64(unsafe.Sizeof(layout) * 8)
-		var sizeFieldBits uint64
-		switch layoutBits { // note: this switch should be resolved at compile time
-		case 16:
-			sizeFieldBits = 4
-		case 32:
-			sizeFieldBits = 5
-		case 64:
-			sizeFieldBits = 6
-		default:
-			runtimePanic("unknown pointer size")
-		}
+// scan an object with this element layout.
+// The starting address must be valid and pointer-aligned.
+// The length is rounded down to a multiple of the element size.
+func (layout gcLayout) scan(start, len uintptr) {
+	switch {
+	case layout == 0:
+		// This is an unknown layout.
+		// Scan conservatively.
+		// NOTE: This is *NOT* equivalent to a slice of pointers on AVR.
+		scanConservative(start, len)
 
-		// Extract values from the bitfields.
-		// See comment at the top of this file for more information.
-		scanner.size = (layout >> 1) & (1<<sizeFieldBits - 1)
-		scanner.bitmap = layout >> (1 + sizeFieldBits)
-	} else {
-		// Layout is stored separately in a global object.
+	case layout&1 != 0:
+		// The layout is stored directly in the integer value.
+		// Extract the bitfields.
+		size := uintptr(layout>>1) & (1<<sizeFieldBits - 1)
+		mask := uintptr(layout) >> (1 + sizeFieldBits)
+
+		// Scan with the extracted mask.
+		scanSimple(start, len, size*unsafe.Alignof(start), mask)
+
+	default:
+		// The layout is stored seperately in a global object.
+		// Extract the size and bitmap.
 		layoutAddr := unsafe.Pointer(layout)
-		scanner.size = *(*uintptr)(layoutAddr)
-		scanner.bitmapAddr = unsafe.Add(layoutAddr, unsafe.Sizeof(uintptr(0)))
+		size := *(*uintptr)(layoutAddr)
+		bitmapPtr := unsafe.Add(layoutAddr, unsafe.Sizeof(uintptr(0)))
+		bitmapLen := (size + 7) / 8
+		bitmap := unsafe.Slice((*byte)(bitmapPtr), bitmapLen)
+
+		// Scan with the bitmap.
+		scanComplex(start, len, size*unsafe.Alignof(start), bitmap)
 	}
-	return
 }
 
-type gcObjectScanner struct {
-	index      uintptr
-	size       uintptr
-	bitmap     uintptr
-	bitmapAddr unsafe.Pointer
+// scanSimple scans an object with an integer bitmask of pointer locations.
+// The starting address must be valid and pointer-aligned.
+func scanSimple(start, len, size, mask uintptr) {
+	for len >= size {
+		// Scan this element.
+		scanWithMask(start, mask)
+
+		// Move to the next element.
+		start += size
+		len -= size
+	}
 }
 
-func (scanner *gcObjectScanner) pointerFree() bool {
-	if scanner.bitmapAddr != nil {
-		// While the format allows for large objects without pointers, this is
-		// optimized by the compiler so if bitmapAddr is set, we know that there
-		// are at least some pointers in the object.
-		return false
-	}
-	// If the bitmap is zero, there are definitely no pointers in the object.
-	return scanner.bitmap == 0
-}
-
-func (scanner *gcObjectScanner) nextIsPointer(word, parent, addrOfWord uintptr) bool {
-	index := scanner.index
-	scanner.index++
-	if scanner.index == scanner.size {
-		scanner.index = 0
-	}
-
-	if !isOnHeap(word) {
-		// Definitely isn't a pointer.
-		return false
-	}
-
-	// Might be a pointer. Now look at the object layout to know for sure.
-	if scanner.bitmapAddr != nil {
-		if (*(*uint8)(unsafe.Add(scanner.bitmapAddr, index/8))>>(index%8))&1 == 0 {
-			return false
+// scanComplex scans an object with a bitmap of pointer locations.
+// The starting address must be valid and pointer-aligned.
+func scanComplex(start, len, size uintptr, bitmap []byte) {
+	for len >= size {
+		// Scan this element.
+		for i, mask := range bitmap {
+			addr := start + 8*unsafe.Alignof(start)*uintptr(i)
+			scanWithMask(addr, uintptr(mask))
 		}
-		return true
-	}
-	if (scanner.bitmap>>index)&1 == 0 {
-		// not a pointer!
-		return false
-	}
 
-	// Probably a pointer.
-	return true
+		// Move to the next element.
+		start += size
+		len -= size
+	}
+}
+
+// scanWithMask scans a portion of an object with a mask of pointer locations.
+// The address must be valid and pointer-aligned.
+func scanWithMask(addr, mask uintptr) {
+	// TODO: use ctz when available
+	for mask != 0 {
+		if mask&1 != 0 {
+			// Load and mark this pointer.
+			root := *(*uintptr)(unsafe.Pointer(addr))
+			markRoot(addr, root)
+		}
+
+		// Move to the next offset.
+		mask >>= 1
+		addr += unsafe.Alignof(addr)
+	}
 }