Merge PR #683: perf: reduce GC scan span and eliminate padding in hot structs

Author: merge-queue-bot

internal/rules/crossfilereferenceintegrity/layout_test.go

@@ -0,0 +1,19 @@
+package crossfilereferenceintegrity
+
+import (
+	"testing"
+	"unsafe"
+)
+
+// TestStructLayout asserts the optimal size for the Rule struct.
+// Moving bool fields to the end (previously between larger fields, wasting
+// padding bytes) reduces size from 128 to 120 bytes and improves cache
+// utilisation across per-Check calls.
+func TestStructLayout(t *testing.T) {
+	got := unsafe.Sizeof(Rule{})
+	const want = uintptr(120)
+	if got != want {
+		t.Errorf("unsafe.Sizeof(Rule{}) = %d; want %d (reorder fields to eliminate padding)",
+			got, want)
+	}
+}

internal/rules/crossfilereferenceintegrity/rule.go

@@ -33,14 +33,16 @@ type LinksConfig struct {
 
 // Rule checks Markdown links for missing target files and missing heading
 // anchors in linked Markdown files.
+// Fields are ordered large-to-small to eliminate bool-induced padding:
+// slices first, then string, then embedded struct, then bools last.
 type Rule struct {
 	Include       []string
 	Exclude       []string
-	Strict        bool
 	Placeholders  []string // placeholder tokens to treat as opaque
-	Wikilinks     bool     // when true, validate Obsidian-style [[...]] targets
 	WikilinkStyle string   // resolution style; only "obsidian" ships today
 	Links         LinksConfig
+	Strict        bool
+	Wikilinks     bool // when true, validate Obsidian-style [[...]] targets
 }
 
 // ID implements rule.Rule.
@@ -61,15 +63,18 @@ func (r *Rule) Category() string { return "link" }
 // relative-link check needs them and the cache is package-scope
 // (cachedAbsRoot) so the cost is paid once across all Files in
 // one run. Plan 195 task 5.
+// checkCtx fields are ordered so pointer-containing fields (maps, then
+// strings) precede the bool, reducing the GC pointer-scan span from 72 to 56.
 type checkCtx struct {
-	f                *lint.File
-	rule             *Rule
+	f           *lint.File
+	rule        *Rule
+	selfAnchors map[string]struct{}
+	anchorCache map[string]map[string]struct{}
+
 	resolvedRoot     string
 	resolvedSiteRoot string
 
-	selfAnchors      map[string]struct{}
 	selfAnchorsBuilt bool
-	anchorCache      map[string]map[string]struct{}
 }
 
 // ensureSelfAnchors lazily builds the heading-anchor set for f.
@@ -268,12 +273,15 @@ func wikilinkRoot(f *lint.File) fs.FS {
 // via f.RunCache), the resolver serves every lookup from that
 // index — turning N files × M targets × workspace-walk into one
 // walk per workspace.
+// wikilinkResolver fields are ordered so the interface (root) and pointer
+// fields (index, memory) precede the strings, reducing GC pointer-scan span
+// from 64 to 56 bytes.
 type wikilinkResolver struct {
 	root   fs.FS
-	from   string
-	style  string
 	index  *linkgraph.WikilinkIndex
 	memory map[string]wikilinkResolveResult
+	from   string
+	style  string
 }
 
 type wikilinkResolveResult struct {
@@ -766,7 +774,10 @@ func (r *Rule) SettingMergeMode(key string) rule.MergeMode {
 	return rule.MergeReplace
 }
 
+// targetFile fields are ordered so the func field (a single pointer) precedes
+// the strings, reducing the GC pointer-scan span from 40 to 32 bytes.
 type targetFile struct {
+	read func() ([]byte, error)
 	// cacheKey is the per-Check cache key (the `cache` map in
 	// anchorsForFile). Prefixed with "os:" or "fs:" so OS and FS
 	// resolutions of the same path do not collide within one
@@ -779,7 +790,6 @@ type targetFile struct {
 	// runCacheKey signals "skip the RunCache slot, use the
 	// per-Check cache only".
 	runCacheKey string
-	read        func() ([]byte, error)
 }
 
 func anchorsForFile(

internal/rules/listscan/listscan.go

@@ -35,24 +35,24 @@ type Item struct {
 	// inside a once-nested list, and so on. It matches the count of
 	// *ast.ListItem ancestors in goldmark's tree.
 	Level int
-	// Ordered reports whether the item belongs to an ordered list.
-	Ordered bool
 	// Number is the literal ordered number written on the marker line
 	// (ordered items only; 0 for bullets).
 	Number int
-	// Marker is the marker byte: '-', '*', or '+' for bullets; '.' or
-	// ')' for ordered items.
-	Marker byte
+	// Ordered reports whether the item belongs to an ordered list.
+	Ordered bool
 	// MultiBlock reports whether the item contains more than one block
 	// child, matching goldmark's isMultiItem (ListItem.ChildCount > 1).
 	MultiBlock bool
+	// Marker is the marker byte: '-', '*', or '+' for bullets; '.' or
+	// ')' for ordered items.
+	Marker byte
 }
 
 // List is one parsed list: a maximal run of sibling items at the same
 // nesting level with the same ordered-ness.
 type List struct {
-	// Ordered reports whether this is an ordered list.
-	Ordered bool
+	// Items holds the list's direct child items in document order.
+	Items []Item
 	// Start is the list's start value: for an ordered list, the literal
 	// number of its first item (matching goldmark list.Start); 0 for an
 	// unordered list.
@@ -65,10 +65,10 @@ type List struct {
 	// LastLine is the 1-based source line of the list's last content
 	// line (including continuation and nested-list lines).
 	LastLine int
+	// Ordered reports whether this is an ordered list.
+	Ordered bool
 	// TopLevel reports whether the list has no *ast.ListItem ancestor.
 	TopLevel bool
-	// Items holds the list's direct child items in document order.
-	Items []Item
 }
 
 // Parse scans lines and returns every list in document order plus a flat
@@ -111,6 +111,11 @@ type frame struct {
 	// blockCount counts the item's block children, mirroring goldmark's
 	// ChildCount used by isMultiItem.
 	blockCount int
+	// childListIndex is the index of the open child list nested directly
+	// under this item, or -1 when none is open. It lets a following
+	// nested marker rejoin the same child list instead of starting a new
+	// one.
+	childListIndex int
 	// pendingBlank records a blank line seen while this item is open but
 	// not yet followed by a continuation; it makes the next content line
 	// start a new block child.
@@ -119,11 +124,6 @@ type frame struct {
 	// item was paragraph text, so a following text line at lower indent
 	// can lazily continue it.
 	inParagraph bool
-	// childListIndex is the index of the open child list nested directly
-	// under this item, or -1 when none is open. It lets a following
-	// nested marker rejoin the same child list instead of starting a new
-	// one.
-	childListIndex int
 	// emptyLine is true while the item has no recorded source line yet
 	// (an empty marker line whose content has not arrived). The first
 	// continuation line that attaches content sets the item's Line.
@@ -133,10 +133,10 @@ type frame struct {
 type parser struct {
 	lines [][]byte
 	lists []List
-	// itemCount counts appended items for sizing the flat slice in Parse.
-	itemCount int
 	// stack holds the currently open list items, outermost first.
 	stack []frame
+	// itemCount counts appended items for sizing the flat slice in Parse.
+	itemCount int
 	// blankRun counts consecutive blank lines pending before the current
 	// line.
 	blankRun int
@@ -360,10 +360,10 @@ func hasMarkerToken(line []byte, indent int) bool {
 
 // markerInfo describes a recognized list-item marker on a line.
 type markerInfo struct {
-	ordered    bool
 	number     int
-	marker     byte
 	contentCol int
+	ordered    bool
+	marker     byte
 	// empty reports that the marker line carries no content after the
 	// marker. goldmark gives such an item no source line of its own, so
 	// its recorded Line is 0 until a continuation line attaches content;

internal/rules/listscan/listscan_layout_test.go

@@ -0,0 +1,29 @@
+package listscan
+
+import (
+	"testing"
+	"unsafe"
+)
+
+// TestStructLayout asserts the optimal field-aligned sizes for the hot-path
+// structs in this package. Each struct is allocated in slices during Parse;
+// tighter layouts reduce per-Check memory and improve cache utilisation.
+// The test fails (red) until fields are reordered to achieve the target sizes.
+func TestStructLayout(t *testing.T) {
+	tests := []struct {
+		name string
+		got  uintptr
+		want uintptr
+	}{
+		{"Item", unsafe.Sizeof(Item{}), 32},
+		{"List", unsafe.Sizeof(List{}), 64},
+		{"frame", unsafe.Sizeof(frame{}), 48},
+		{"markerInfo", unsafe.Sizeof(markerInfo{}), 24},
+	}
+	for _, tc := range tests {
+		if tc.got != tc.want {
+			t.Errorf("unsafe.Sizeof(%s{}) = %d; want %d (reorder fields to eliminate padding)",
+				tc.name, tc.got, tc.want)
+		}
+	}
+}

internal/rules/tablefmt/layout_test.go

@@ -0,0 +1,24 @@
+package tablefmt
+
+import (
+	"testing"
+)
+
+// TestStripPrefixNoAlloc asserts that stripPrefix performs zero heap
+// allocations when given a non-empty prefix. The previous implementation
+// allocated string(line) and []byte(…) — two allocs per table row.
+func TestStripPrefixNoAlloc(t *testing.T) {
+	if raceEnabled {
+		t.Skip("alloc gate skipped under -race")
+	}
+	line := []byte("> | col | other |")
+	prefix := "> "
+	var sink []byte
+	allocs := testing.AllocsPerRun(100, func() {
+		sink = stripPrefix(line, prefix)
+	})
+	_ = sink
+	if allocs != 0 {
+		t.Errorf("stripPrefix allocated %.0f times; want 0", allocs)
+	}
+}

internal/rules/tablefmt/tablefmt.go

@@ -15,9 +15,10 @@ import (
 
 // Violation describes a single table whose source formatting differs
 // from the canonical layout produced by this package.
+// Message (a string, with a pointer) precedes StartLine to minimise GC scan span.
 type Violation struct {
-	StartLine int    // 1-based line number of the table's first row
 	Message   string // diagnostic message including the first differing row
+	StartLine int    // 1-based line number of the table's first row
 }
 
 // Config controls how tables are formatted.
@@ -171,18 +172,23 @@ func normalizeConfig(cfg Config) Config {
 }
 
 // table represents a parsed markdown table with its source location.
+// Fields are ordered so slices and the string (all pointer-bearing) precede
+// the scalar int, and the string sits between the two slices so all
+// pointers are contiguous — reducing GC pointer-scan span from 56 to 48.
 type table struct {
-	startLine int      // 1-based line number of the first row
 	rawLines  [][]byte // raw source lines (including prefix)
 	prefix    string   // blockquote/list prefix (e.g. "> ", "  ")
 	rows      []row    // parsed rows (header, separator, data)
+	startLine int      // 1-based line number of the first row
 }
 
 // row is a single table row with its cells.
+// Fields are ordered so pointer-containing fields (slices) precede the bool,
+// minimising the GC pointer-scan span.
 type row struct {
 	cells       []string // trimmed cell contents
-	isSeparator bool     // true for the separator row (|---|---|)
 	alignments  []align  // alignment per column (only for separator row)
+	isSeparator bool     // true for the separator row (|---|---|)
 }
 
 // align represents column alignment in a table.
@@ -434,13 +440,16 @@ func detectPrefix(line []byte) string {
 }
 
 // stripPrefix removes the detected prefix from a line.
+// Uses the compiler-optimised string(line[:n]) == prefix comparison
+// (same pattern as tableformat/structure.go rowContent) to avoid
+// allocating a temporary string copy of the full line.
 func stripPrefix(line []byte, prefix string) []byte {
-	if prefix == "" {
+	plen := len(prefix)
+	if plen == 0 || len(line) < plen {
 		return line
 	}
-	s := string(line)
-	if strings.HasPrefix(s, prefix) {
-		return []byte(s[len(prefix):])
+	if string(line[:plen]) == prefix {
+		return line[plen:]
 	}
 	return line
 }

internal/rules/tableformat/layout_test.go

@@ -0,0 +1,19 @@
+package tableformat
+
+import (
+	"testing"
+	"unsafe"
+)
+
+// TestRuleFieldOrder asserts that the Style string field comes first in the
+// MDS025 Rule struct so that the GC pointer-scan span is minimised to 8 bytes
+// (just the string data pointer at offset 0) rather than 24 bytes (when Style
+// sat after two int fields). The test fails (red) until fields are reordered.
+func TestRuleFieldOrder(t *testing.T) {
+	got := unsafe.Offsetof(Rule{}.Style)
+	const want = uintptr(0)
+	if got != want {
+		t.Errorf("unsafe.Offsetof(Rule.Style) = %d; want %d (move Style first to minimise GC scan)",
+			got, want)
+	}
+}

internal/rules/tableformat/rule.go

@@ -28,10 +28,12 @@ func init() {
 // Rule gates table well-formedness: edge-pipe style (MD055), column
 // count vs the header (MD056), surrounding blank lines (MD058), and
 // the column-alignment / padding pass that gives the rule its name.
+// Style (a string, containing a pointer) sits first to keep the GC
+// pointer-scan span at 8 bytes instead of 24.
 type Rule struct {
-	Pad            int // spaces on each side of cell content
-	SeparatorStyle tablefmt.SeparatorStyle
 	Style          string // edge-pipe style: one of the Style* constants
+	Pad            int    // spaces on each side of cell content
+	SeparatorStyle tablefmt.SeparatorStyle
 }
 
 // ID implements rule.Rule.

internal/rules/toc/layout_test.go

@@ -0,0 +1,20 @@
+package toc
+
+import (
+	"testing"
+	"unsafe"
+)
+
+// TestRuleFieldOrder asserts that the engine pointer field comes first in the
+// MDS038 Rule struct. The current layout puts engine after engineOnce (a
+// sync.Once = 12 bytes with no pointers, padded to 16 to align the pointer),
+// forcing GC to scan 24 bytes. Moving engine first reduces the GC pointer-scan
+// span to 8 bytes. The test fails (red) until fields are reordered.
+func TestRuleFieldOrder(t *testing.T) {
+	got := unsafe.Offsetof(Rule{}.engine)
+	const want = uintptr(0)
+	if got != want {
+		t.Errorf("unsafe.Offsetof(Rule.engine) = %d; want %d (move engine first to minimise GC scan)",
+			got, want)
+	}
+}

internal/rules/toc/rule.go

@@ -23,9 +23,11 @@ func init() {
 // singleton and the LSP server may call Check from concurrent
 // goroutines, where a plain check-then-set on the engine field
 // would race.
+// engine sits before engineOnce so the GC pointer-scan span is 8 bytes
+// (just the pointer) rather than 24 (pointer buried after sync.Once).
 type Rule struct {
-	engineOnce sync.Once
 	engine     *gensection.Engine
+	engineOnce sync.Once
 }
 
 // ID implements rule.Rule.