Skip to content

Commit 3cf569c

Browse files
jedudenclaude
andauthored
docs: add High-Performance Go contributor guide (#373)
* docs: add high-performance Go contributor guide A new docs/development/high-performance-go.md page captures the benchmark→profile→fix loop, the patterns that keep rules inside the ≤ 10 allocs/Check budget, and the anti-patterns that have already cost the project real CPU and GC time. The catalogs in docs/development/index.md, CLAUDE.md, AGENTS.md, and .github/copilot-instructions.md regenerated to pick it up. https://claude.ai/code/session_01N1pHzrCYHHBKqWSH5rez36 * docs: ground the high-performance-go intro in real numbers "Multiplies by tens of thousands" was hand-wavy. Name the multiplier (the 600-file Large benchmark gate × one alloc per Check) and what an O(n) rescan actually costs in seconds. https://claude.ai/code/session_01N1pHzrCYHHBKqWSH5rez36 * docs: lead with patterns-first; fold war stories into patterns Three changes to docs/development/high-performance-go.md: - Reframe the process: "apply best-practice patterns first, then measure, then fix what's still hot" — not "measure first, never optimize from a hunch". The known wins (pre- size, hoist regex, return nil) don't need a benchmark to justify; the speculative ones do. - Generalize the loop: drop the rule-specific MDS024 and `./internal/rules/...` example so the playbook applies to parser, engine, and CLI code, not only rules. - Replace the `b.Loop()` syntax tip with the project's actual practice: write benchmarks that always run and gate their budget inline with `b.Fatalf`. - Extract the two war-story learnings into a new "Skip work you don't need" pattern — memoize per-input computations; gate expensive analyzers behind a cheap pre-check. https://claude.ai/code/session_01N1pHzrCYHHBKqWSH5rez36 * docs: address review feedback on high-performance-go PR #373 review found several inaccuracies. Fixes: - `[]T{}` does not unconditionally "allocate a header"; the header is a value type and only escapes in some cases. Rephrased both the pattern bullet and the avoid-table row around the project convention (nil vs empty) and the conditional escape. - `bufio.Scanner` is risky for huge inputs (default 64 KB token limit). Table now recommends `bufio.Reader`, with Scanner as a fallback when `Scanner.Buffer` is tuned. - The `-alloc_objects` cell in the profile table mixed a go-test source with a pprof view flag. Collapsed Alloc and In-use into one Memory row with a note explaining the two pprof views. - Dropped the "perfsprint linter flags this" claim — the repo's .golangci.yml does not enable perfsprint. - `go tool fieldalignment -fix ./...` is not a stock tool invocation; replaced with a pointer to the analyzer in golang.org/x/tools. - `go vet -shadow` is not a real flag in modern Go; dropped from the Tooling section. Tooling now points at the project's actual `go tool golangci-lint run` gate and lists perfsprint/prealloc/gocritic as opt-in extensions on top. https://claude.ai/code/session_01N1pHzrCYHHBKqWSH5rez36 * docs: address Copilot review feedback (round 2) Three more nits from the re-review: - `[]T{}` "24-byte header" claim was architecture-specific and conflated escape with allocation. Reworded around the observable nil-vs-empty difference (tests, JSON, reflect) and the project convention. - `sync.Once` is "a single atomic load" — dropped "relaxed" since Go's atomics are sequentially consistent, not C/C++-style relaxed. - "each `+` allocates" was misleading; the compiler can concatenate multiple operands in one alloc. Reworded the table cell around the loop case (per-iteration alloc, quadratic total). https://claude.ai/code/session_01N1pHzrCYHHBKqWSH5rez36 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cab3ea8 commit 3cf569c

5 files changed

Lines changed: 315 additions & 0 deletions

File tree

.github/copilot-instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ row: "- [{summary}](../{filename})"
4040
- [SOLID and clean architecture patterns for the mdsmith VS Code extension at editors/vscode/.](../docs/development/architecture/typescript.md)
4141
- [Codecov coverage gate and CI status checks.](../docs/development/coverage.md)
4242
- [Where to place Markdown files and documentation types.](../docs/development/file-placement.md)
43+
- [Process and patterns for keeping mdsmith's Go core fast: the benchmark→profile→fix loop, the patterns to reach for, and the anti-patterns that have already cost the project real CPU and GC time.](../docs/development/high-performance-go.md)
4344
- [Build commands, project layout, code style, test fixtures, coverage gate, and merge conflicts.](../docs/development/index.md)
4445
- [The pkg/markdown public package: parse, produce, and its compatibility policy.](../docs/development/markdown-library.md)
4546
- [Label-driven merge queue workflow using jeduden/merge-queue-action.](../docs/development/merge-queue.md)
@@ -184,6 +185,7 @@ row: "- [{title}](../docs/development/{filename})"
184185
- [Architecture principles](../docs/development/architecture/index.md)
185186
- [Coverage Gate](../docs/development/coverage.md)
186187
- [File Placement](../docs/development/file-placement.md)
188+
- [High-Performance Go](../docs/development/high-performance-go.md)
187189
- [Merge Queue](../docs/development/merge-queue.md)
188190
- [PR Fixup Workflow](../docs/development/pr-fixup-workflow.md)
189191
- [Public Markdown Library](../docs/development/markdown-library.md)

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ row: "- [{summary}]({filename})"
4646
- [SOLID and clean architecture patterns for the mdsmith VS Code extension at editors/vscode/.](docs/development/architecture/typescript.md)
4747
- [Codecov coverage gate and CI status checks.](docs/development/coverage.md)
4848
- [Where to place Markdown files and documentation types.](docs/development/file-placement.md)
49+
- [Process and patterns for keeping mdsmith's Go core fast: the benchmark→profile→fix loop, the patterns to reach for, and the anti-patterns that have already cost the project real CPU and GC time.](docs/development/high-performance-go.md)
4950
- [Build commands, project layout, code style, test fixtures, coverage gate, and merge conflicts.](docs/development/index.md)
5051
- [The pkg/markdown public package: parse, produce, and its compatibility policy.](docs/development/markdown-library.md)
5152
- [Label-driven merge queue workflow using jeduden/merge-queue-action.](docs/development/merge-queue.md)
@@ -189,6 +190,7 @@ row: "- [{title}](docs/development/{filename})"
189190
- [Architecture principles](docs/development/architecture/index.md)
190191
- [Coverage Gate](docs/development/coverage.md)
191192
- [File Placement](docs/development/file-placement.md)
193+
- [High-Performance Go](docs/development/high-performance-go.md)
192194
- [Merge Queue](docs/development/merge-queue.md)
193195
- [PR Fixup Workflow](docs/development/pr-fixup-workflow.md)
194196
- [Public Markdown Library](docs/development/markdown-library.md)

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ row: "- [{summary}]({filename})"
3232
- [SOLID and clean architecture patterns for the mdsmith VS Code extension at editors/vscode/.](docs/development/architecture/typescript.md)
3333
- [Codecov coverage gate and CI status checks.](docs/development/coverage.md)
3434
- [Where to place Markdown files and documentation types.](docs/development/file-placement.md)
35+
- [Process and patterns for keeping mdsmith's Go core fast: the benchmark→profile→fix loop, the patterns to reach for, and the anti-patterns that have already cost the project real CPU and GC time.](docs/development/high-performance-go.md)
3536
- [Build commands, project layout, code style, test fixtures, coverage gate, and merge conflicts.](docs/development/index.md)
3637
- [The pkg/markdown public package: parse, produce, and its compatibility policy.](docs/development/markdown-library.md)
3738
- [Label-driven merge queue workflow using jeduden/merge-queue-action.](docs/development/merge-queue.md)
@@ -175,6 +176,7 @@ row: "- [{title}](docs/development/{filename})"
175176
- [Architecture principles](docs/development/architecture/index.md)
176177
- [Coverage Gate](docs/development/coverage.md)
177178
- [File Placement](docs/development/file-placement.md)
179+
- [High-Performance Go](docs/development/high-performance-go.md)
178180
- [Merge Queue](docs/development/merge-queue.md)
179181
- [PR Fixup Workflow](docs/development/pr-fixup-workflow.md)
180182
- [Public Markdown Library](docs/development/markdown-library.md)
Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
---
2+
title: High-Performance Go
3+
summary: >-
4+
Process and patterns for keeping mdsmith's Go core fast:
5+
the benchmark→profile→fix loop, the patterns to reach
6+
for, and the anti-patterns that have already cost the
7+
project real CPU and GC time.
8+
---
9+
# High-Performance Go
10+
11+
mdsmith's hot path is the rule set running over every file
12+
in the workspace. The Large benchmark gate parses 600 files
13+
through the full rule set. One extra alloc per `Check` is
14+
tens of thousands of extra allocs per `mdsmith check`. One
15+
accidental O(n) rescan inside a rule turns a 0.8 s run into
16+
several seconds. This page is the contributor playbook for
17+
keeping that path fast.
18+
19+
The per-rule ≤ 10 alloc ceiling and the tiered CI gates
20+
live elsewhere:
21+
22+
- [Allocation Budget](index.md#allocation-budget) — the
23+
rule and how to verify it.
24+
- [Markdown linter benchmark](../research/benchmarks/README.md)
25+
— corpus benchmarks, gates, and the
26+
`profile.sh` / `MDSMITH_CPUPROFILE` workflow.
27+
28+
This page is the methodology behind those budgets and the
29+
patterns that keep us inside them.
30+
31+
## Process
32+
33+
**Apply best-practice patterns first. Then measure. Then
34+
fix what is still hot.** The patterns in
35+
[Patterns to apply](#patterns-to-apply) are known wins
36+
across the Go core — pre-size a slice, hoist a regex,
37+
return `nil` for an empty result. Use them by default;
38+
they cost nothing.
39+
40+
Past that, profile before you rewrite — a flat profile in
41+
your suspected hot path stops a useless fix. The loop:
42+
43+
1. **State the goal numerically.** "Function X under N
44+
allocs/call on representative input" — not "make it
45+
faster". Name the symbol (function, rule, package) and
46+
the input that hits its hot frame.
47+
2. **Lock in a baseline** by running the package's
48+
existing benchmarks multiple times:
49+
50+
```bash
51+
go test -run=^$ -bench=. -count=10 -benchmem \
52+
./path/to/package > old.txt
53+
```
54+
55+
3. **Profile** the baseline. CPU profile if you don't know
56+
the bottleneck; alloc profile if `b.ReportAllocs` shows
57+
allocations; trace if latency is bad but CPU is idle.
58+
4. **Change one thing.** Re-run the same benchmark, same
59+
count.
60+
5. **Decide with `benchstat`,** not eyeballs. Re-run the
61+
benchmark into `new.txt`, then:
62+
63+
```bash
64+
benchstat old.txt new.txt
65+
```
66+
67+
`~` in the delta column means no significant change.
68+
p < 0.05 with a meaningful effect size is the bar.
69+
70+
**Write benchmarks that always run.** Put the bench next
71+
to the code. Pin its budget inline with `b.Fatalf` on
72+
overshoot, as `BenchmarkRule_MDS024` does. CI then catches
73+
the next slip on its own.
74+
75+
### Which profile answers which question
76+
77+
| Profile | Source | Question |
78+
|---------|--------------------------------------|-----------------------------------|
79+
| CPU | `-cpuprofile cpu.out` | Where is time going? |
80+
| Memory | `-memprofile m.out` | What allocates and what is live |
81+
| Block | `runtime.SetBlockProfileRate(1)` | Where do goroutines wait? |
82+
| Mutex | `runtime.SetMutexProfileFraction(1)` | Who holds contended locks? |
83+
| Trace | `-trace trace.out` | Scheduler / GC / syscall timeline |
84+
85+
View memory profiles with `go tool pprof -alloc_objects`
86+
(every allocation, including freed) or `-inuse_objects`
87+
(what is resident now).
88+
89+
mdsmith ships a profile hook for the CLI
90+
(`internal/profiling/profiling.go`):
91+
92+
```bash
93+
MDSMITH_CPUPROFILE=cpu.out mdsmith check .
94+
go tool pprof -http=:8080 cpu.out
95+
```
96+
97+
No CLI flag on purpose — the command line stays
98+
byte-identical to production. For diffs, use
99+
`go tool pprof -base=old.prof new.prof`.
100+
101+
### Escape analysis
102+
103+
Before adding `sync.Pool`, read what the compiler already
104+
does:
105+
106+
```bash
107+
go build -gcflags="-m=2" ./pkg/markdown 2>&1 | grep escape
108+
```
109+
110+
Common causes of escape: returning a pointer to a local;
111+
storing a value in `interface{}` / `any`; capturing a
112+
variable in a closure that outlives the frame; slice or
113+
map growth past a compile-time-known size. The stack costs
114+
nothing; the heap costs an alloc plus future GC scan.
115+
116+
### Profile-guided optimization
117+
118+
PGO has been GA since Go 1.21 and lands 2–14% wins on real
119+
binaries. For mdsmith:
120+
121+
1. Run `mdsmith check` over a representative corpus with
122+
`MDSMITH_CPUPROFILE=cmd/mdsmith/default.pgo`.
123+
2. `go build` picks the file up automatically.
124+
3. Refresh after major rule changes.
125+
126+
Worth it for release builds; not worth it for one-off
127+
debug builds.
128+
129+
## Patterns to apply
130+
131+
The list below extends the project's existing
132+
[Allocation Budget](index.md#allocation-budget) rules.
133+
Reach for these first.
134+
135+
### Allocations
136+
137+
Each removed alloc in a per-file `Check` saves one alloc
138+
per workspace file.
139+
140+
- **Pre-size slices.** `make([]T, 0, n)` when `n` is
141+
known. `append` doubles capacity up to ~1024 then grows
142+
~25%, copying each step.
143+
- **Reuse loop-local buffers.** `buf = buf[:0]` clears
144+
length, keeps capacity. See `extractTextBufPool` in
145+
`internal/mdtext/mdtext.go`.
146+
- **`sync.Pool` for transient state.** Best for
147+
expensive, short-lived state (line scanners, AST
148+
scratch). Always reset before `Put`; pool entries can
149+
be reaped by GC without notice. Examples:
150+
`internal/punkt/tokenizer.go`,
151+
`internal/schema/validate_content.go`.
152+
- **Return `nil`, not `[]T{}`.** Project convention.
153+
`nil` and a non-nil empty slice are distinguishable in
154+
tests, JSON, and `reflect`; sticking to `nil` for "no
155+
result" keeps callers uniform.
156+
- **Compile regexes at package scope.**
157+
`var foo = regexp.MustCompile(…)`. Compiling inside a
158+
hot function builds the NFA every call.
159+
160+
### Strings and bytes
161+
162+
- **Stay in `[]byte`.** Each `string(b)` allocates and
163+
copies. `bytes.IndexByte` and `bytes.Contains` are
164+
SIMD-accelerated on amd64; faster than `strings.*` once
165+
you already have bytes.
166+
- **`strings.Builder` over `+`.** Concatenation in a loop
167+
allocates a new backing array each time. Call
168+
`Grow(n)` first if you know the final size.
169+
- **`strconv` over `fmt.Sprintf`.** `strconv.Itoa(n)` is
170+
~3× faster than `fmt.Sprintf("%d", n)` and skips
171+
reflection.
172+
- **`strings.EqualFold` for case-insensitive compare.**
173+
One pass, no allocation; beats `ToLower` + `==`.
174+
- **`unsafe.String` / `unsafe.Slice` (Go 1.20+)** for
175+
zero-copy `[]byte``string`. The caller must guarantee
176+
the source isn't mutated and outlives the view. Use
177+
sparingly, with a comment naming the invariant.
178+
179+
### Fixed-string search beats regex
180+
181+
`bytes.IndexByte('#')` is a hardware-assisted single-byte
182+
scan. `regexp.MustCompile("#").FindIndex` builds an NFA
183+
and walks it. For anything expressible as a literal,
184+
substring, or prefix/suffix check, skip `regexp`.
185+
186+
### Data structures
187+
188+
- **Fixed-size arrays beat slices** when the size is
189+
known — no header, no escape.
190+
- **`map[K]struct{}` for sets** — zero-byte value type.
191+
- **Sorted slice + binary search** beats a map for
192+
n < ~100, thanks to cache locality. Benchmark at your
193+
real n.
194+
- **Swiss tables in Go 1.24+.** Free 30–60% map speedup
195+
and up to 70% map-memory reduction; no code change
196+
needed.
197+
198+
### Struct layout
199+
200+
- **Order fields large-to-small** to minimize padding.
201+
The `fieldalignment` analyzer in `golang.org/x/tools`
202+
flags layouts with wasted bytes and can rewrite them.
203+
- **Hot/cold split.** Frequently-read fields in one
204+
struct, rarely-read in another behind a pointer.
205+
Better cache utilization in the hot path.
206+
- **Prefer `[]Foo` over `[]*Foo`.** A value slice is one
207+
GC-scanned allocation with zero internal pointers; the
208+
pointer slice forces N pointer scans every cycle.
209+
210+
### Skip work you don't need
211+
212+
The cheapest call is the one you never make. Two real
213+
mdsmith wins live here:
214+
215+
- **Memoize per-input computations.** When a helper runs
216+
many times over the same `*lint.File`, cache the result
217+
on the File. The cached newline index in
218+
`lint.(*File).LineOfOffset` replaced an O(n) rescan per
219+
call — ~24% of `check` CPU on long prose before the
220+
fix.
221+
- **Gate expensive analyzers behind a cheap pre-check.**
222+
An upper- or lower-bound check that proves the
223+
expensive path can't produce a diagnostic lets you
224+
skip it. MDS024's guard skips the sentence tokenizer
225+
when no paragraph can violate either limit — ~2 GB of
226+
saved allocations on the 600-file gate corpus.
227+
228+
### Inlining
229+
230+
The inliner has a budget (~80 nodes per function). Keep
231+
hot functions tiny so they inline; outline the slow path
232+
into a separate function. The canonical model is
233+
`sync.Mutex.Lock`: the uncontended CAS inlines; the
234+
contended slow path is a separate function.
235+
236+
Inspect with `go build -gcflags="-m=2"` and look for
237+
`can inline foo` / `inlining call to foo`.
238+
239+
### Concurrency
240+
241+
- **`sync/atomic`** for one-word flags and counters
242+
(`atomic.Bool`, `atomic.Int64`, `atomic.Pointer[T]`).
243+
- **`sync.Once`** for lazy init. After the first call,
244+
it costs a single atomic load.
245+
- **`sync.Mutex`** for >1-word critical sections.
246+
Default choice. Cheaper than `sync.RWMutex` under low
247+
contention.
248+
- **Channels for handoff or backpressure**, not for
249+
protecting a single variable — under contention a
250+
channel is orders of magnitude slower than a mutex.
251+
- **`errgroup.SetLimit(n)`** for bounded fan-out. Size
252+
by bottleneck: `runtime.NumCPU()` for CPU-bound, much
253+
higher for I/O-bound.
254+
- **Every goroutine must exit on `ctx.Done()`.** Test
255+
with `go.uber.org/goleak`.
256+
257+
## Patterns to avoid
258+
259+
| Avoid | Why | Use instead |
260+
|--------------------------------------|--------------------------------------------------------------------------------|-----------------------------------------------------------------|
261+
| `fmt.Sprintf("%d", n)` in hot paths | reflection, ~3× slower | `strconv.Itoa(n)` |
262+
| `s + s2 + s3` in a loop | repeated concatenation allocates a new backing array per iteration (quadratic) | `strings.Builder` with `Grow` |
263+
| `append` growing without `make` | doubling-copy cost | pre-size with known cap |
264+
| `defer` in a tight loop | in-loop `defer` falls off the open-coded fast path | hoist or inline cleanup |
265+
| return `[]T{}` for "no result" | non-uniform with project convention; the empty literal allocates if it escapes | return `nil` |
266+
| `any` / `interface{}` in hot paths | boxing forces heap copy; defeats devirtualization | concrete types, or generics |
267+
| `reflect` in hot paths | type-info walks, allocations | code-gen or hand-roll |
268+
| `regexp` for a literal | NFA build + walk | `bytes.Contains` / `strings.HasPrefix` |
269+
| goroutine-per-item | scheduler & memory pressure | `errgroup.SetLimit(n)` |
270+
| channel for one shared variable | scheduler hop, allocations | `sync.Mutex` or atomic |
271+
| copying a `sync.Mutex` | silent lock breakage | pass `*Mutex`; `go vet` catches some |
272+
| `defer mu.Unlock()` in tiny section | defer cost dwarfs the body | inline unlock when no panic path |
273+
| `log.Printf` per item in a hot loop | format + lock + I/O | sample, or batch outside the loop |
274+
| `time.Now()` in a tight loop | wall + monotonic read each call | read once, use `time.Since` |
275+
| `os.ReadFile` on huge inputs | one giant alloc, all resident | `bufio.Reader` (or `bufio.Scanner` with `Scanner.Buffer` tuned) |
276+
| `context.Background()` deep in calls | loses cancellation | propagate caller's `ctx` |
277+
278+
## Tooling
279+
280+
- **`go tool golangci-lint run`** — the project's lint
281+
gate. Consider enabling `perfsprint`, `prealloc`, and
282+
gocritic's performance group on top of what
283+
`.golangci.yml` already runs.
284+
- **`benchstat`** for "this is faster" claims.
285+
- **`go.uber.org/goleak`** to assert no leftover
286+
goroutines in tests.
287+
- **`go tool pprof -base=old.prof new.prof`** to diff
288+
profiles before and after a change.
289+
290+
## References
291+
292+
- [Dave Cheney — High Performance Go Workshop][cheney]
293+
- [Damian Gryski — go-perfbook][perfbook]
294+
- [Go blog — Profile-Guided Optimization in Go 1.21][pgo]
295+
- [Go blog — Faster Go maps with Swiss Tables][swiss]
296+
- [Go blog — `testing.B.Loop`][bloop]
297+
- [Filippo Valsorda — Efficient Go APIs with the inliner][filippo]
298+
- [Eli Bendersky — Common pitfalls in Go benchmarking][eli]
299+
- [PlanetScale — Generics can make your Go code slower][ps]
300+
301+
[cheney]: https://dave.cheney.net/high-performance-go-workshop/dotgo-paris.html
302+
[perfbook]: https://github.com/dgryski/go-perfbook/blob/master/performance.md
303+
[pgo]: https://go.dev/blog/pgo
304+
[swiss]: https://go.dev/blog/swisstable
305+
[bloop]: https://go.dev/blog/testing-b-loop
306+
[filippo]: https://words.filippo.io/efficient-go-apis-with-the-inliner/
307+
[eli]: https://eli.thegreenplace.net/2023/common-pitfalls-in-go-benchmarking/
308+
[ps]: https://planetscale.com/blog/generics-can-make-your-go-code-slower

docs/development/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ row: "- [{title}]({filename})"
1919
- [Architecture principles](architecture/index.md)
2020
- [Coverage Gate](coverage.md)
2121
- [File Placement](file-placement.md)
22+
- [High-Performance Go](high-performance-go.md)
2223
- [Merge Queue](merge-queue.md)
2324
- [PR Fixup Workflow](pr-fixup-workflow.md)
2425
- [Public Markdown Library](markdown-library.md)

0 commit comments

Comments
 (0)