Skip to content

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

Merged
jeduden merged 4 commits into
mainfrom
claude/kind-darwin-sinfn3
Jun 22, 2026
Merged

perf: reduce GC scan span and eliminate padding in hot structs#683
jeduden merged 4 commits into
mainfrom
claude/kind-darwin-sinfn3

Conversation

@jeduden

@jeduden jeduden commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Summary

Addresses struct layout violations identified by auditing against docs/development/high-performance-go.md. Each fix reduces GC pointer-scan span or eliminates alignment padding in structs that are allocated per-lint-call, reducing GC pressure on the hot path. All fixes follow Red/Green TDD: failing layout/alloc test → struct reorder or function rewrite → passing test → commit.

Fixes

  • tablefmt.stripPrefix — zero-alloc rewrite: The previous implementation called string(line) and []byte(…), causing 2 heap allocations per table row. Replaced with a string(line[:plen]) == prefix slice comparison the compiler optimises to a zero-alloc memcmp. Guarded by TestStripPrefixNoAlloc (skipped under -race where instrumentation inflates alloc counts).

  • listscan structs — padding elimination: Reordered Item (40→32 bytes), List (72→64 bytes), frame (56→48 bytes), and markerInfo (40→24 bytes) to group bools together at the end, eliminating bool-induced alignment padding. Asserted by TestStructLayout via unsafe.Sizeof.

  • crossfilereferenceintegrity structs — GC span + padding: Reordered Rule (128→120 bytes, bools moved last), checkCtx (GC span 72→56, maps before strings), wikilinkResolver (GC span 64→56, interface+pointer before strings), and targetFile (GC span 40→32, func field first). Asserted by TestStructLayout via unsafe.Sizeof.

  • tableformat.Rule — GC scan span: Moved Style string first so the GC pointer-scan span is 8 bytes (string data pointer at offset 0) rather than 24 bytes (string after two int fields). Asserted by TestRuleFieldOrder via unsafe.Offsetof.

  • toc.Rule — GC scan span: Moved engine *gensection.Engine before engineOnce sync.Once so the GC pointer-scan span is 8 bytes (pointer at offset 0) rather than 24 bytes (pointer buried after the 12-byte sync.Once padded to 16 for alignment). Asserted by TestRuleFieldOrder via unsafe.Offsetof.

Post-review fixes (3 rounds of /code-review --effort xhigh)

  • Added raceEnabled guard to tablefmt/layout_test.gotesting.AllocsPerRun returns inflated counts under -race due to instrumentation overhead; existing tablefmt tests use the same guard pattern.
  • Corrected sync.Once size in toc/layout_test.go comment: sync.Once is 12 bytes in Go 1.25, padded to 16 to align the subsequent pointer, giving a 24-byte scan span in the old layout.
  • Fixed stale present-tense comment in crossfilereferenceintegrity/layout_test.go that described the struct as "currently packs bool fields" after the reorder was already applied.

Test plan

  • go test ./internal/rules/listscan/...TestStructLayout passes at 32/64/48/24 bytes
  • go test ./internal/rules/tablefmt/...TestStripPrefixNoAlloc allocates 0
  • go test ./internal/rules/crossfilereferenceintegrity/...TestStructLayout passes at 120 bytes
  • go test ./internal/rules/tableformat/...TestRuleFieldOrder passes at offset 0
  • go test ./internal/rules/toc/...TestRuleFieldOrder passes at offset 0
  • go test ./... — full suite green

Reference: docs/development/high-performance-go.md

🤖 Generated with Claude Code

https://claude.ai/code/session_015pKxVjp82WDkwB4MKNNKM8

Audit against docs/development/high-performance-go.md identified padding
waste and GC pointer-scan bloat in five packages. Each fix is guarded by a
red/green test (TestStructLayout, TestRuleFieldOrder, TestStripPrefixNoAlloc)
that fails before the reorder and passes after.

1. listscan: reorder Item/List/frame/markerInfo/parser fields
   - Item 40→32 bytes (8 bytes saved per item in per-file slices)
   - List 72→64 bytes (8 bytes saved per list)
   - frame 56→48 bytes (8 bytes saved per open stack entry)
   - markerInfo 40→24 bytes (16 bytes saved per parsed marker line)
   - parser: group pointer-bearing slices first, reducing GC scan range
   Five rules (MDS014/016/045/046/061) benefit on every file with lists.

2. tablefmt: eliminate double string/byte allocation in stripPrefix
   - Previous impl: string(line) assigned to variable + []byte(s[len:]) =
     2 allocs per table row when a blockquote/list prefix is present.
   - Fix: string(line[:plen]) == prefix (compiler-optimised, 0 allocs) and
     return the sub-slice line[plen:] (no copy).
   - Same pattern already used in tableformat/structure.go rowContent.
   MDS025 carries the highest grandfathered alloc budget (60); every
   removed alloc per row compounds across multi-row tables.

3. tablefmt: GC scan reduction for Violation, table, row structs
   - Violation: string (pointer) moved before int → scan 16→8 bytes.
   - table: string moved between the two slices → scan 56→48 bytes.
   - row: isSeparator bool moved after alignments slice → scan 40→32 bytes.

4. crossfilereferenceintegrity: struct field reorderings
   - Rule struct: bool fields Strict/Wikilinks moved after slices/string/Links;
     size 128→120 bytes (8 bytes saved per Rule instance).
   - checkCtx: maps grouped before strings, bool last; GC scan 72→56 bytes.
   - wikilinkResolver: pointer fields first, strings last; GC scan 64→56 bytes.
   - targetFile: func field first, strings after; GC scan 40→32 bytes.

5. tableformat + toc: GC scan reduction for Rule structs
   - MDS025 Rule: Style string moved first; GC scan 24→8 bytes.
   - MDS038 Rule: engine pointer moved before engineOnce; GC scan 24→8 bytes.

Ref: docs/development/high-performance-go.md (struct layout, GC scan, allocs)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pKxVjp82WDkwB4MKNNKM8
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.62%. Comparing base (7d37ea5) to head (e0efed0).

Additional details and impacted files
Components Coverage Δ
Go 98.61% <100.00%> (ø)
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.

claude added 3 commits June 22, 2026 20:32
Under -race, race instrumentation inflates AllocsPerRun counts, causing
the zero-alloc assertion to fail spuriously. All three existing alloc
tests in tablefmt_test.go already skip under -race; align this new test
with that pattern. Also move the escape sink from package-level to
function-local to avoid cross-test variable sharing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pKxVjp82WDkwB4MKNNKM8
sync.Once is 12 bytes (atomic.Bool 4 + sync.Mutex 8), not 16. The old
layout still produces a 24-byte GC scan span because 4 bytes of alignment
padding push the engine pointer to offset 16.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pKxVjp82WDkwB4MKNNKM8
The comment described the struct as "currently packs bool fields between
larger fields" — present tense that was accurate before the reorder but
misleads future readers once the fix is applied. Rewrite to past tense.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pKxVjp82WDkwB4MKNNKM8
@jeduden jeduden changed the title perf: fix struct field alignment in 5 hot-path packages perf: reduce GC scan span and eliminate padding in hot structs Jun 22, 2026
@jeduden 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 Jun 22, 2026
@jeduden

jeduden commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

🟢 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

jeduden commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

🔵 Merge Queue — CI running

Merged into batch branch merge-queue/batch-683-1782161485. View CI run.

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

@jeduden
jeduden merged commit 1599c9f into main Jun 22, 2026
32 checks passed
@jeduden jeduden removed the queue:active Applied automatically when a PR is in an active batch label Jun 22, 2026
@jeduden

jeduden commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

Merge Queue — merged

This PR landed on main via commit 1599c9f. CI run that validated the merge.

Next: Done — nothing more to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants