Merge PR #765: perf: fix top 5 high-performance-go.md violations (struct layout + byte scans)

Author: merge-queue-bot

docs/reference/cli.md

@@ -154,13 +154,13 @@ path (`····^`) pointing to the exact column.
 [
   {
     "file": "README.md",
-    "line": 10,
-    "column": 81,
     "rule": "MDS001",
     "name": "line-length",
     "severity": "error",
     "message": "line too long (120 > 80)",
     "source_lines": ["Previous line.", "Another context.", "The long line...", "Next.", "Another."],
+    "line": 10,
+    "column": 81,
     "source_start_line": 8
   }
 ]

internal/archetype/gensection/engine.go

@@ -147,18 +147,14 @@ func ReplaceContent(f *lint.File, mp MarkerPair, content string) []byte {
 	return result
 }
 
-// SplitLines splits source into lines (like bytes.Split but returns [][]byte).
+// SplitLines splits source into lines on []byte instead of
+// hand-rolling a byte-by-byte scan: bytes.Split uses the same
+// SIMD-accelerated IndexByte the standard library relies on
+// everywhere else in mdsmith, and it pre-sizes the result in one
+// pass instead of growing it via repeated append. See
+// docs/development/high-performance-go.md "Strings and bytes".
 func SplitLines(source []byte) [][]byte {
-	var lines [][]byte
-	start := 0
-	for i, b := range source {
-		if b == '\n' {
-			lines = append(lines, source[start:i])
-			start = i + 1
-		}
-	}
-	lines = append(lines, source[start:])
-	return lines
+	return bytes.Split(source, []byte{'\n'})
 }
 
 // EnsureTrailingNewline appends \n if s does not already end with \n.

internal/archetype/gensection/engine_test.go

@@ -491,6 +491,60 @@ func TestSplitLines_Empty(t *testing.T) {
 	require.Len(t, lines, 1, "expected 1 line, got %d", len(lines))
 }
 
+// splitLinesAllocBudget pins SplitLines to a single allocation per
+// call. A hand-rolled append loop over each '\n' reallocates its
+// backing array as the line count grows past each capacity doubling;
+// bytes.Split pre-counts separators in one pass and allocates the
+// result slice exactly once. See
+// docs/development/high-performance-go.md "Strings and bytes".
+const splitLinesAllocBudget = 1
+
+// representativeMarkdownSource is a 50-line body sized like a real
+// Markdown file passed through Engine.Fix — enough lines that a
+// growth-only append loop needs several reallocations, but not an
+// artificial stress size.
+func representativeMarkdownSource() []byte {
+	var b strings.Builder
+	for i := 0; i < 50; i++ {
+		b.WriteString("this is a representative line of markdown prose\n")
+	}
+	return []byte(b.String())
+}
+
+// TestSplitLines_AllocBudget pins the per-call allocation count so a
+// regression back to the manual append loop fails CI.
+func TestSplitLines_AllocBudget(t *testing.T) {
+	if testing.Short() {
+		t.Skip("alloc gate skipped in -short mode")
+	}
+	src := representativeMarkdownSource()
+	allocs := testing.AllocsPerRun(100, func() {
+		_ = SplitLines(src)
+	})
+	require.LessOrEqualf(t, allocs, float64(splitLinesAllocBudget),
+		"SplitLines allocs/op = %.0f, budget = %d", allocs, splitLinesAllocBudget)
+}
+
+// BenchmarkSplitLines reports allocs/op for SplitLines on a
+// representative 50-line body and fails the run if it regresses past
+// splitLinesAllocBudget, matching the project's "benchmarks that
+// always run" convention.
+func BenchmarkSplitLines(b *testing.B) {
+	src := representativeMarkdownSource()
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		_ = SplitLines(src)
+	}
+	b.StopTimer()
+
+	allocs := testing.AllocsPerRun(100, func() {
+		_ = SplitLines(src)
+	})
+	if allocs > float64(splitLinesAllocBudget) {
+		b.Fatalf("SplitLines allocs/op = %.0f, budget = %d", allocs, splitLinesAllocBudget)
+	}
+}
+
 func TestParseColumnConfig_Basic(t *testing.T) {
 	raw := map[string]any{
 		"desc": map[string]any{

internal/lsp/protocol.go

@@ -177,13 +177,20 @@ const (
 )
 
 // Diagnostic is the LSP wire shape produced by the server.
+//
+// Fields are ordered pointer-containing (string/slice/pointer/struct-
+// with-pointer-field) first, then scalar (int) last, matching
+// internal/lint.Diagnostic's layout. Go's GC computes a struct's
+// ptrdata as the offset through the last pointer-containing field;
+// this type is built once per diagnostic on the LSP's keystroke hot
+// path (every publishDiagnostics call), so keeping the scalar fields
+// out of that span matters here. See
+// docs/development/high-performance-go.md "Struct layout".
 type Diagnostic struct {
-	Range    Range              `json:"range"`
-	Severity DiagnosticSeverity `json:"severity,omitempty"`
-	Code     string             `json:"code,omitempty"`
-	Source   string             `json:"source,omitempty"`
-	Message  string             `json:"message"`
-	Data     *diagnosticData    `json:"data,omitempty"`
+	Code    string          `json:"code,omitempty"`
+	Source  string          `json:"source,omitempty"`
+	Message string          `json:"message"`
+	Data    *diagnosticData `json:"data,omitempty"`
 	// RelatedInformation surfaces secondary locations (plan 230): for
 	// MDS020, the proto.md / kind-file line that declares the violated
 	// constraint, which the editor renders as a navigable entry.
@@ -194,6 +201,9 @@ type Diagnostic struct {
 	// to its documentation. Href must be an http(s) URL per the LSP
 	// spec; clients render it next to the code.
 	CodeDescription *codeDescription `json:"codeDescription,omitempty"`
+
+	Range    Range              `json:"range"`
+	Severity DiagnosticSeverity `json:"severity,omitempty"`
 }
 
 // diagnosticRelatedInformation is one entry of Diagnostic.relatedInformation

internal/lsp/server_codeaction.go

@@ -1,6 +1,7 @@
 package lsp
 
 import (
+	"bytes"
 	"encoding/json"
 	"strings"
 
@@ -425,14 +426,11 @@ func documentEndPosition(source []byte) (int, int) {
 	}
 	if source[len(source)-1] == '\n' {
 		// Count newlines; the position past the final \n is the
-		// one-past-the-end line, character 0.
-		nl := 0
-		for _, b := range source {
-			if b == '\n' {
-				nl++
-			}
-		}
-		return nl, 0
+		// one-past-the-end line, character 0. bytes.Count uses the
+		// same SIMD-accelerated scan as bytes.IndexByte instead of a
+		// scalar per-byte Go loop — see
+		// docs/development/high-performance-go.md "Strings and bytes".
+		return bytes.Count(source, []byte{'\n'}), 0
 	}
 	// No trailing newline: end at last line's UTF-16 length. source
 	// is non-empty here (checked above), so splitLines always yields

internal/lsp/server_test.go

@@ -2124,6 +2124,39 @@ func TestDocumentEndPositionEmpty(t *testing.T) {
 	assert.Equal(t, 0, endChar)
 }
 
+// representativeLSPDocument is a 2000-line trailing-newline body sized
+// like a real document passed through fullFileEdit on every "fix all"
+// whole-document code action.
+func representativeLSPDocument() []byte {
+	var b strings.Builder
+	for i := 0; i < 2000; i++ {
+		b.WriteString("this is a representative line of markdown prose\n")
+	}
+	return []byte(b.String())
+}
+
+// BenchmarkDocumentEndPosition pins documentEndPosition to zero
+// allocations on the trailing-newline path so a regression back to an
+// allocating implementation fails CI. The newline count itself uses
+// bytes.Count's SIMD-accelerated scan instead of a scalar per-byte Go
+// loop; see docs/development/high-performance-go.md "Strings and
+// bytes".
+func BenchmarkDocumentEndPosition(b *testing.B) {
+	src := representativeLSPDocument()
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		_, _ = documentEndPosition(src)
+	}
+	b.StopTimer()
+
+	allocs := testing.AllocsPerRun(100, func() {
+		_, _ = documentEndPosition(src)
+	})
+	if allocs > 0 {
+		b.Fatalf("documentEndPosition allocs/op = %.0f, budget = 0", allocs)
+	}
+}
+
 func TestReloadConfigEmptyRoot(t *testing.T) {
 	t.Parallel()
 	s := New(Options{Reader: nil, Writer: io.Discard})

internal/lsp/structlayout_test.go

@@ -0,0 +1,12 @@
+package lsp
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/jeduden/mdsmith/internal/structlayout"
+)
+
+func TestDiagnostic_PointerFieldsPrecedeScalars(t *testing.T) {
+	structlayout.AssertPointerFieldsFirst(t, reflect.TypeOf(Diagnostic{}))
+}

internal/output/json.go

@@ -10,36 +10,45 @@ import (
 // JSONFormatter outputs diagnostics as a JSON array.
 type JSONFormatter struct{}
 
+// Fields are ordered pointer-containing (string/slice/pointer) first,
+// then scalar (int/bool) last, matching internal/lint.Diagnostic's
+// layout. Go's GC computes a struct's ptrdata as the offset through
+// the last pointer-containing field; one of these is built per
+// diagnostic on every `--format json` run. See
+// docs/development/high-performance-go.md "Struct layout".
 type jsonDiagnostic struct {
-	File            string           `json:"file"`
-	Line            int              `json:"line"`
-	Column          int              `json:"column"`
-	Rule            string           `json:"rule"`
-	Name            string           `json:"name"`
-	Severity        string           `json:"severity"`
-	Message         string           `json:"message"`
-	SourceLines     []string         `json:"source_lines,omitempty"`
-	SourceStartLine int              `json:"source_start_line,omitempty"`
-	Explanation     *jsonExplanation `json:"explanation,omitempty"`
-	// Deprecated and ReplacedBy mirror lint.Diagnostic's plan-136
-	// fields so CI scripts can route a deprecation warning without
-	// scanning the message body. Both are omitempty so non-
-	// deprecation diagnostics stay unchanged on the wire.
-	Deprecated bool   `json:"deprecated,omitempty"`
+	File        string           `json:"file"`
+	Rule        string           `json:"rule"`
+	Name        string           `json:"name"`
+	Severity    string           `json:"severity"`
+	Message     string           `json:"message"`
+	SourceLines []string         `json:"source_lines,omitempty"`
+	Explanation *jsonExplanation `json:"explanation,omitempty"`
+	// ReplacedBy mirrors lint.Diagnostic's plan-136 field so CI
+	// scripts can route a deprecation warning without scanning the
+	// message body. omitempty so non-deprecation diagnostics stay
+	// unchanged on the wire.
 	ReplacedBy string `json:"replaced_by,omitempty"`
 	// RelatedLocations mirrors lint.Diagnostic's plan-230 field so CI
 	// scripts can read the schema-constraint location without parsing
 	// the message. omitempty so diagnostics that carry none stay
 	// unchanged on the wire. The rule-doc URL is not emitted here — it
 	// is derivable from the `rule` field and is an editor (LSP) concern.
 	RelatedLocations []jsonRelatedLocation `json:"related_locations,omitempty"`
+
+	Line            int  `json:"line"`
+	Column          int  `json:"column"`
+	SourceStartLine int  `json:"source_start_line,omitempty"`
+	Deprecated      bool `json:"deprecated,omitempty"`
 }
 
+// File and Message (pointer-containing) precede Line and Column
+// (scalar) for the same GC-ptrdata reason as jsonDiagnostic above.
 type jsonRelatedLocation struct {
 	File    string `json:"file,omitempty"`
+	Message string `json:"message"`
 	Line    int    `json:"line,omitempty"`
 	Column  int    `json:"column,omitempty"`
-	Message string `json:"message"`
 }
 
 type jsonExplanation struct {

internal/output/json_test.go

@@ -170,12 +170,12 @@ func TestJSONFormatter_ExactOutput(t *testing.T) {
 	expected := `[
   {
     "file": "README.md",
-    "line": 10,
-    "column": 5,
     "rule": "MDS001",
     "name": "line-length",
     "severity": "error",
-    "message": "line too long (120 \u003e 80)"
+    "message": "line too long (120 \u003e 80)",
+    "line": 10,
+    "column": 5
   }
 ]
 `

internal/output/structlayout_test.go

@@ -0,0 +1,16 @@
+package output
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/jeduden/mdsmith/internal/structlayout"
+)
+
+func TestJSONDiagnostic_PointerFieldsPrecedeScalars(t *testing.T) {
+	structlayout.AssertPointerFieldsFirst(t, reflect.TypeOf(jsonDiagnostic{}))
+}
+
+func TestJSONRelatedLocation_PointerFieldsPrecedeScalars(t *testing.T) {
+	structlayout.AssertPointerFieldsFirst(t, reflect.TypeOf(jsonRelatedLocation{}))
+}