perf: fix top 5 high-performance-go.md violations (struct layout + byte scans) by jeduden · Pull Request #765 · jeduden/mdsmith

jeduden · 2026-07-23T20:31:38Z

Summary

An audit against docs/development/high-performance-go.md (four parallel scans covering allocation anti-patterns, the "patterns to avoid" table, struct layout/data structures, and strings/bytes/concurrency) surfaced several real violations. This PR fixes the top 5, ranked by hot-path frequency, safety, and testability:

internal/lsp.Diagnostic — Range/Severity (scalars) were declared before pointer-containing fields (Code, Source, Message, Data, RelatedInformation, CodeDescription). Go's GC computes a struct's ptrdata as the span from offset 0 through the last pointer-containing field, so those scalars sat inside the scanned span for nothing. This type's own doc comment calls it out as the "keystroke hot path" — built once per diagnostic on every publishDiagnostics call.
pkg/mdsmith.Diagnostic / RelatedLocation — same interleaving defect on the public Session API surface (WASM bindings, Obsidian plugin, any embedder), built once per diagnostic in toDiagnostics/toRelatedLocations. The sibling internal/lint.Diagnostic already carries this exact fix; these two were missed.
internal/output.jsonDiagnostic / jsonRelatedLocation — same defect on the CI / --format json output path, built once per diagnostic on every mdsmith check --format json run.
gensection.SplitLines — hand-rolled byte-by-byte scan for '\n' building the result via unsized append, instead of bytes.Split. This is a regression of a pattern the project already fixed once (internal/lsp/diagnostics.go's splitLines uses bytes.Split for the same reason). Runs on every Engine.Fix pass for <?include?>/<?catalog?>-style directives. Measured: 6 allocs/op → 1 alloc/op on a representative 50-line body; 13 allocs/186µs → 1 alloc/43µs on a 2000-line stress input.
LSP documentEndPosition — manual per-byte newline-counting loop instead of bytes.Count, on every LSP "fix all" whole-document code action. Measured: ~63µs/op → ~1.6–2µs/op on a representative 2000-line document, allocations unchanged at zero.

All five map directly to sections of docs/development/high-performance-go.md: "Struct layout" (group pointer fields first, scalars last — GC ptrdata spans through the last pointer field) for 1–3, and "Strings and bytes" (bytes.IndexByte/bytes.Count over a hand-rolled byte loop) for 4–5.

Approach

Each fix follows red/green TDD:

Struct layout fixes (1–3): added a structlayout_test.go per package using the project's existing internal/structlayout.AssertPointerFieldsFirst reflection helper (already used for lint.Diagnostic, schema.ScopeMatch, and others) — confirmed red against the current field order, then reordered fields to green.
Byte-scan fixes (4–5): added an allocation-budget test/benchmark (testing.AllocsPerRun, b.Fatalf on overshoot, matching the BenchmarkRule_MDS024 convention) — confirmed red against the manual loop, then swapped in the stdlib call to green.

Note: reordering jsonDiagnostic's Go struct fields changes encoding/json's emitted key order (Go encodes in declaration order), so TestJSONFormatter_ExactOutput is updated to match. JSON key order carries no semantic meaning, no reference doc commits to a specific order, and the CLI's own e2e JSON tests parse into a map rather than asserting exact text — only that one pinned unit test needed updating.

Test plan

go build ./...
go test ./... (full suite green)
go vet ./...
go tool -modfile=tools/go.mod golangci-lint run ./... (0 issues)
internal/integration allocation-budget gate passes
go run ./cmd/mdsmith check . (0 failures)
New structlayout_test.go / alloc-budget tests confirmed red against the pre-fix code, green after

🤖 Generated with Claude Code

Generated by Claude Code

SplitLines built its result via append in a byte-by-byte scan for '\n', growing the slice through several capacity doublings and never vectorizing the scan. bytes.Split uses the same SIMD-accelerated IndexByte the rest of the codebase already relies on and pre-sizes the result in one pass. On a 50-line representative body this drops 6 allocs/op to 1; on a 2000-line stress input, 13 allocs/186us to 1 alloc/43us. Per docs/development/high-performance-go.md "Strings and bytes": bytes.IndexByte over a hand-rolled byte loop. This is a regression of a pattern the project already fixed once (internal/lsp/diagnostics.go splitLines uses bytes.Split for the same reason). Adds TestSplitLines_AllocBudget and BenchmarkSplitLines pinning the 1-alloc budget so a regression fails CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNiF3CVZG2NCizbsTgur4V

… loop documentEndPosition's trailing-newline branch counted '\n' with a scalar per-byte Go for-range loop. bytes.Count uses the same SIMD-accelerated scan bytes.IndexByte relies on, and this function runs on every LSP "fix all" whole-document code action (fullFileEdit) over the complete document buffer. On a representative 2000-line document this drops ~63us/op to ~1.6-2us/op with allocations unchanged at zero. Per docs/development/high-performance-go.md "Strings and bytes": bytes.IndexByte (and its relatives, including bytes.Count) over a hand-rolled byte loop for scanning a large Source. Adds BenchmarkDocumentEndPosition, pinning the zero-alloc property so a regression fails CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNiF3CVZG2NCizbsTgur4V

internal/lsp.Diagnostic placed Range (a scalar struct) and Severity (an int) before Code/Source/Message/Data/RelatedInformation/ CodeDescription. Go's GC ptrdata for a struct spans from offset 0 through the end of the last pointer-containing field, so those two scalars sat inside the scanned span for no reason. This is the type's own doc comment's "keystroke hot path" — built once per diagnostic on every publishDiagnostics call — and internal/lint.Diagnostic already carries the same fix with the same rationale. Per docs/development/high-performance-go.md "Struct layout": order fields large-to-small, group pointer fields first and scalars last. Adds TestDiagnostic_PointerFieldsPrecedeScalars using the project's existing internal/structlayout.AssertPointerFieldsFirst helper, the same pattern already used for lint.Diagnostic, schema.ScopeMatch, and others. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNiF3CVZG2NCizbsTgur4V

… scalars pkg/mdsmith.Diagnostic interleaved Line/Column/SourceStartLine/ Deprecated (scalars) among its string/slice/pointer fields; its sibling RelatedLocation interleaved Line/Column before Message. Neither got the pointer-fields-first fix that internal/lint.Diagnostic and internal/lint.RelatedLocation already carry for the same reason: Go's GC ptrdata for a struct spans from offset 0 through the last pointer-containing field, so a scalar declared before that point sits inside the scanned span for nothing. toDiagnostics/toRelatedLocations build one of these per lint finding on every Session.Check/Session.Fix call — the public engine API that the WASM bindings, the Obsidian plugin, and any other embedder of pkg/mdsmith go through. Per docs/development/high-performance-go.md "Struct layout": order fields large-to-small, group pointer fields first and scalars last. Adds structlayout_test.go with the same structlayout.AssertPointerFieldsFirst pattern already used for lint.Diagnostic, schema.ScopeMatch, and others. JSON field tags are unchanged, so the wire shape (CLI --format json / LSP / WASM host) is unaffected by the in-memory reorder. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNiF3CVZG2NCizbsTgur4V

… before scalars internal/output.jsonDiagnostic interleaved Line/Column/ SourceStartLine/Deprecated (scalars) among its string/slice/pointer fields; jsonRelatedLocation interleaved Line/Column before Message. Same defect and same fix as internal/lint.Diagnostic and pkg/mdsmith.Diagnostic: Go's GC ptrdata for a struct spans from offset 0 through the last pointer-containing field, so a scalar declared before that point sits inside the scanned span for nothing. One of these is built per diagnostic on every `--format json` run (CI pipelines, `mdsmith check --format json`). Per docs/development/high-performance-go.md "Struct layout": order fields large-to-small, group pointer fields first and scalars last. Adds structlayout_test.go with the project's existing structlayout.AssertPointerFieldsFirst pattern. Reordering the Go struct fields changes encoding/json's emitted key order (Go encodes struct fields in declaration order), so TestJSONFormatter_ExactOutput is updated to match; JSON key order carries no semantic meaning and no reference doc commits to a specific order, and the CLI's own e2e JSON tests parse into a map rather than asserting exact text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNiF3CVZG2NCizbsTgur4V

codecov · 2026-07-23T20:32:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.69%. Comparing base (d4af5d5) to head (fbc7cd5).
⚠️ Report is 38 commits behind head on main.

Additional details and impacted files

Components	Coverage Δ
Go	`98.69% <100.00%> (-0.01%)`	⬇️
TypeScript	`99.54% <ø> (ø)`

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The struct-layout fix in internal/output.jsonDiagnostic (previous commit) reordered Go struct fields to group pointer-containing fields before scalars, which changes encoding/json's emitted key order. This reference page's --format json example still showed the pre-fix order. Verified against actual `mdsmith check --format json` output. Found by an independent adversarial review pass on PR #765. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNiF3CVZG2NCizbsTgur4V

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

jeduden · 2026-07-25T19:12:24Z

🟢 Merge Queue — picked up

This PR is in the queue and will be batched with other queue-labelled PRs.

Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run.

jeduden · 2026-07-25T19:14:10Z

🔵 Merge Queue — CI running

Merged into batch branch merge-queue/batch-760-1785006745 alongside #760, #763, #766. View CI run.

Next: No action needed — you'll be notified when CI completes.

jeduden · 2026-07-25T19:19:18Z

✅ Merge Queue — merged

This PR landed on main via commit 94af540. CI run that validated the merge.

Next: Done — nothing more to do here.

claude added 5 commits July 23, 2026 20:21

jeduden marked this pull request as ready for review July 23, 2026 20:44

Copilot AI review requested due to automatic review settings July 23, 2026 20:44

Copilot AI reviewed Jul 23, 2026

jeduden requested a review from Copilot July 23, 2026 20:44

Copilot AI reviewed Jul 23, 2026

jeduden added queue Add to a PR to enqueue it queue:active Applied automatically when a PR is in an active batch and removed queue Add to a PR to enqueue it labels Jul 25, 2026

jeduden mentioned this pull request Jul 25, 2026

fix(cli): skip non-Markdown files named explicitly on check/fix #760

Merged

This was referenced Jul 25, 2026

chore(deps): bump markdownlint-cli2 from 0.23.0 to 0.23.1 in /docs/research/benchmarks/npm #763

Merged

docs(security): 2026-07-24 post-audit diff review — MDS072 SSRF #766

Merged

jeduden merged commit dba714d into main Jul 25, 2026
35 checks passed

jeduden removed the queue:active Applied automatically when a PR is in an active batch label Jul 25, 2026

This was referenced Jul 26, 2026

perf: audit against high-performance-go.md, fix 5 measured hot paths #774

Draft

perf: fix top 5 high-performance-go.md violations found by codebase audit #782

Draft

perf: fix top 5 high-performance-go.md violations from a 3-agent audit #783

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

perf: fix top 5 high-performance-go.md violations (struct layout + byte scans)#765
jeduden merged 6 commits into
mainfrom
claude/kind-darwin-qokafg

jeduden commented Jul 23, 2026

Uh oh!

codecov Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

jeduden commented Jul 25, 2026

Uh oh!

jeduden commented Jul 25, 2026

Uh oh!

Uh oh!

jeduden commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jeduden commented Jul 23, 2026

Summary

Approach

Test plan

Uh oh!

codecov Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

jeduden commented Jul 25, 2026

Uh oh!

jeduden commented Jul 25, 2026

Uh oh!

Uh oh!

jeduden commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jul 23, 2026 •

edited

Loading