Merge PR #833: perf: fix top 5 high-performance-go violations (struct layout + allocations)

Author: merge-queue-bot

internal/rules/linkvalidity/layout_test.go

@@ -0,0 +1,21 @@
+package linkvalidity
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/jeduden/mdsmith/internal/structlayout"
+)
+
+// TestRevMatchFieldOrder guards the GC ptrdata region of revMatch:
+// the pointer-containing text and url byte slices must precede the
+// scalar col0 and matchEnd ints. Go's GC ptrdata for a struct spans
+// from offset 0 through the last pointer-containing field's pointer
+// word; with the scalars first (the prior order), ptrdata still had
+// to cover both slices, so grouping the slices first instead shrinks
+// ptrdata from 48 bytes to 32 on a 64-bit platform. See
+// docs/development/high-performance-go.md "Struct layout". The test
+// fails (red) until col0 and matchEnd are moved after text and url.
+func TestRevMatchFieldOrder(t *testing.T) {
+	structlayout.AssertPointerFieldsFirst(t, reflect.TypeOf(revMatch{}))
+}

internal/rules/linkvalidity/rule.go

@@ -219,10 +219,10 @@ func isInlineNode(n ast.Node) bool {
 // --- reversed-link scan (MD011) ---
 
 type revMatch struct {
-	col0     int // 0-based byte index of '(' within the line
-	matchEnd int // exclusive byte index just past ']' within the line
 	text     []byte
 	url      []byte
+	col0     int // 0-based byte index of '(' within the line
+	matchEnd int // exclusive byte index just past ']' within the line
 }
 
 // reversedNeedle is the two-byte sequence every reversedRe match must

internal/rules/noinlinehtml/alloc_test.go

@@ -0,0 +1,34 @@
+package noinlinehtml
+
+import "testing"
+
+// allocBudgetExtractTagUppercase is the per-call ceiling for
+// extractTag on a mixed-case tag name. tagNameRe.FindSubmatch pays
+// one allocation for the match slice; the old
+// strings.ToLower(string(m[1])) paid two more whenever m[1] contained
+// an uppercase byte — the string(m[1]) copy, then strings.ToLower's
+// own buffer since it cannot return the input unchanged.
+// asciiLowerTag folds those two into one allocation, bringing the
+// total from 3 down to 2 — see
+// docs/development/high-performance-go.md "Allocations."
+const allocBudgetExtractTagUppercase = 2
+
+func TestExtractTagUppercaseAllocBudget(t *testing.T) {
+	if testing.Short() {
+		t.Skip("alloc gate skipped in -short mode")
+	}
+	if raceEnabled {
+		t.Skip("alloc gate skipped under -race")
+	}
+	raw := []byte("<DIV class=\"x\">")
+	_ = extractTag(raw) // warm up regex program cache
+	allocs := testing.AllocsPerRun(200, func() {
+		_ = extractTag(raw)
+	})
+	t.Logf("extractTag(uppercase) allocs/op = %.0f (budget = %d)", allocs, allocBudgetExtractTagUppercase)
+	if allocs > float64(allocBudgetExtractTagUppercase) {
+		t.Fatalf("extractTag(uppercase) allocs/op = %.0f, budget = %d: lowercase the "+
+			"matched bytes directly instead of string(m[1]) followed by strings.ToLower",
+			allocs, allocBudgetExtractTagUppercase)
+	}
+}

internal/rules/noinlinehtml/race_off_test.go

@@ -0,0 +1,5 @@
+//go:build !race
+
+package noinlinehtml
+
+const raceEnabled = false

internal/rules/noinlinehtml/race_on_test.go

@@ -0,0 +1,5 @@
+//go:build race
+
+package noinlinehtml
+
+const raceEnabled = true

internal/rules/noinlinehtml/rule.go

@@ -284,7 +284,30 @@ func extractTag(raw []byte) string {
 	if m == nil {
 		return ""
 	}
-	return strings.ToLower(string(m[1]))
+	return asciiLowerTag(m[1])
+}
+
+// asciiLowerTag returns the lowercased tag name for b in one
+// allocation. tagNameRe only matches [a-zA-Z][a-zA-Z0-9-]*, so b is
+// always ASCII: strings.ToLower(string(b)) allocates the string(b)
+// copy and then, for any uppercase input, a second buffer inside
+// strings.ToLower — see docs/development/high-performance-go.md's
+// allocation guidance on avoiding a redundant intermediate copy.
+func asciiLowerTag(b []byte) string {
+	for _, c := range b {
+		if 'A' <= c && c <= 'Z' {
+			var sb strings.Builder
+			sb.Grow(len(b))
+			for _, c := range b {
+				if 'A' <= c && c <= 'Z' {
+					c += 'a' - 'A'
+				}
+				sb.WriteByte(c)
+			}
+			return sb.String()
+		}
+	}
+	return string(b)
 }
 
 func isClosingTag(raw []byte) bool {

internal/rules/samefileanchor/layout_test.go

@@ -0,0 +1,21 @@
+package samefileanchor
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/jeduden/mdsmith/internal/structlayout"
+)
+
+// TestAnchorCheckerFieldOrder guards the GC ptrdata region of
+// anchorChecker: built (a bool, no pointers) must come after the
+// pointer-containing r, f, slugs, and diags fields. Go's GC ptrdata
+// for a struct spans from offset 0 through the end of the last
+// pointer-containing field, so a scalar declared before diags (the
+// last pointer field) costs GC scan bytes for nothing — a saving of
+// one word on a 64-bit platform. See
+// docs/development/high-performance-go.md "Struct layout". The test
+// fails (red) until built is moved after diags.
+func TestAnchorCheckerFieldOrder(t *testing.T) {
+	structlayout.AssertPointerFieldsFirst(t, reflect.TypeOf(anchorChecker{}))
+}

internal/rules/samefileanchor/rule.go

@@ -72,8 +72,8 @@ type anchorChecker struct {
 	r     *Rule
 	f     *lint.File
 	slugs map[string]struct{}
-	built bool
 	diags []lint.Diagnostic
+	built bool
 }
 
 // visit records a diagnostic when n is a same-file fragment link whose

internal/rules/tableformat/checkcolumncount_alloc_test.go

@@ -0,0 +1,42 @@
+package tableformat
+
+import (
+	"testing"
+
+	"github.com/jeduden/mdsmith/internal/lint"
+	"github.com/stretchr/testify/require"
+)
+
+// TestCheckColumnCountCompliant_NoAlloc pins that checkColumnCount
+// returns nil, with zero allocations, when every row's cell count
+// matches the header. The old make([]lint.Diagnostic, 0,
+// len(t.rows)-1) allocated a slice on every call and then discarded
+// it via `if len(diags) == 0 { return nil }` on this common
+// compliant-table path — see docs/development/high-performance-go.md's
+// "Return nil, not []T{}" pattern.
+func TestCheckColumnCountCompliant_NoAlloc(t *testing.T) {
+	f, err := lint.NewFile("t.md", []byte("| a | b |\n|---|---|\n| 1 | 2 |\n"))
+	require.NoError(t, err)
+	tables := findStructureTables(f.Lines, structureSkipFunc(f))
+	require.Len(t, tables, 1)
+	tbl := tables[0]
+
+	if got := checkColumnCount(f, tbl, "MDS025", "table-format"); got != nil {
+		t.Fatalf("checkColumnCount on a compliant table = %#v, want nil", got)
+	}
+
+	if testing.Short() {
+		t.Skip("alloc gate skipped in -short mode")
+	}
+	if raceEnabled {
+		t.Skip("alloc gate skipped under -race")
+	}
+	allocs := testing.AllocsPerRun(200, func() {
+		_ = checkColumnCount(f, tbl, "MDS025", "table-format")
+	})
+	t.Logf("checkColumnCount(compliant) allocs/op = %.0f", allocs)
+	if allocs > 0 {
+		t.Fatalf("checkColumnCount(compliant) allocs/op = %.0f, want 0: allocate diags "+
+			"lazily via append instead of an eager make() that gets discarded", allocs)
+	}
+}

internal/rules/tableformat/structure.go

@@ -211,17 +211,17 @@ func checkPipeStyle(f *lint.File, t tableBlock, style, ruleID, ruleName string)
 
 func checkColumnCount(f *lint.File, t tableBlock, ruleID, ruleName string) []lint.Diagnostic {
 	want := t.rows[0].cells
-	diags := make([]lint.Diagnostic, 0, len(t.rows)-1)
+	// Most tables are column-count compliant; an eager make() here
+	// would allocate a slice only to discard it on that common path,
+	// so diags starts nil and grows lazily instead.
+	var diags []lint.Diagnostic
 	for _, row := range t.rows[1:] {
 		if row.cells == want {
 			continue
 		}
 		diags = append(diags, structureDiag(f, row.lineNum, 1, ruleID, ruleName,
 			fmt.Sprintf("table column count; expected %d, got %d", want, row.cells)))
 	}
-	if len(diags) == 0 {
-		return nil
-	}
 	return diags
 }