Skip to content

Commit 81e544e

Browse files
author
merge-queue-bot
committed
Merge PR #424: Plan 215: Audit AST-walking rules and rewrite the ones that only need f.Lines
2 parents b2b2d68 + 72f0d6d commit 81e544e

10 files changed

Lines changed: 2236 additions & 211 deletions

File tree

PLAN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ footer: |
142142
| 214 | 🔳 | sonnet | [MDS019 catalog: CUE-expression row templates](plan/214_catalog-cue-row-expressions.md) |
143143
| 214 || opus | [Obsidian plugin via hand-rolled LSP bridge](plan/214_obsidian-plugin.md) |
144144
| 215 | 🔲 | opus | [mdsmith public engine API and WASM bindings](plan/215_engine-api-wasm.md) |
145-
| 215 | 🔲 | opus | [Audit AST-walking rules and rewrite the ones that only need f.Lines](plan/215_lines-only-rule-audit.md) |
145+
| 215 | | opus | [Audit AST-walking rules and rewrite the ones that only need f.Lines](plan/215_lines-only-rule-audit.md) |
146146
| 216 || opus | [Per-document parse cache for the LSP, keyed by version](plan/216_lsp-parse-cache.md) |
147147
| 217 | 🔲 | opus | [Obsidian plugin (WASM runtime)](plan/217_obsidian-plugin.md) |
148148
| 218 | 🔲 | sonnet | [Finish MD054 link-image-style coverage in MDS068](plan/218_finish-md054-link-image-style.md) |

docs/development/high-performance-go.md

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,11 @@ accidental O(n) rescan inside a rule turns a 0.8 s run into
1616
several seconds. This page is the contributor playbook for
1717
keeping that path fast.
1818

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.
19+
This page is the methodology behind the project's budgets.
20+
The ≤ 10 alloc ceiling lives in
21+
[Allocation Budget](index.md#allocation-budget). The corpus
22+
gates and `MDSMITH_CPUPROFILE` workflow live in the
23+
[benchmark notes](../research/benchmarks/README.md).
3024

3125
## Process
3226

@@ -72,6 +66,24 @@ to the code. Pin its budget inline with `b.Fatalf` on
7266
overshoot, as `BenchmarkRule_MDS024` does. CI then catches
7367
the next slip on its own.
7468

69+
Opt-in rules (those returning
70+
`EnabledByDefault() == false`) skip `BenchmarkCheckCorpus*`,
71+
so `perrule_bench_test.go` is their only time gate. It pins
72+
each a `perRuleBenchBudget` row — `Time` near 5× the logged
73+
baseline, `Allocs` near baseline plus `max(20%, 4)`.
74+
`optInRules` finds new opt-in rules from `rule.All()`, so
75+
the gate fails with "no pinned budget" until you add the
76+
row. It times parse+Check together: parse dwarfs Check, but
77+
constant parse cost lets the sum still catch a regression.
78+
Allocs stay the tight gate (subtracted, deterministic).
79+
80+
To decide whether a rule needs the AST,
81+
`testdata/rule_walk_audit.json` records each rule's class
82+
(plan 215). **Category A** rules scan `f.Lines` with no
83+
AST. **Category B** rules drive `f.ProseRanges()` instead
84+
of re-implementing fences. **AST-required** rules keep the
85+
tree. No AST-walking rule is cleanly Category A today.
86+
7587
### Which profile answers which question
7688

7789
| Profile | Source | Question |
@@ -116,15 +128,11 @@ nothing; the heap costs an alloc plus future GC scan.
116128
### Profile-guided optimization
117129

118130
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.
131+
binaries. Run `mdsmith check` over a representative corpus
132+
with `MDSMITH_CPUPROFILE=cmd/mdsmith/default.pgo`; `go
133+
build` then picks the file up automatically. Refresh after
134+
major rule changes. Worth it for release builds, not for
135+
one-off debug builds.
128136

129137
## Patterns to apply
130138

@@ -256,36 +264,28 @@ Inspect with `go build -gcflags="-m=2"` and look for
256264

257265
## Patterns to avoid
258266

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` |
267+
Every [Patterns to apply](#patterns-to-apply) rule has an
268+
inverse anti-pattern. Reaching for `fmt.Sprintf`, `+` in a
269+
loop, an un-`make`d `append`, `[]T{}`, `any`, `regexp` for
270+
a literal, goroutine-per-item, or a channel for one
271+
variable are the obvious ones. A few more are below.
272+
273+
| Avoid | Why | Use instead |
274+
| ------------------------------------ | ---------------------------------- | ---------------------------------- |
275+
| `defer` in a tight loop | falls off the open-coded fast path | hoist or inline cleanup |
276+
| `reflect` in hot paths | type-info walks, allocations | code-gen or hand-roll |
277+
| `log.Printf` per item in a hot loop | format + lock + I/O | sample, or batch outside the loop |
278+
| `time.Now()` in a tight loop | wall + monotonic read each call | read once, use `time.Since` |
279+
| `os.ReadFile` on huge inputs | one giant alloc, all resident | `bufio.Reader` with a tuned buffer |
280+
| `context.Background()` deep in calls | loses cancellation | propagate caller's `ctx` |
277281

278282
## Tooling
279283

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.
284+
See [Process](#process) for the `benchstat`, `pprof`, and
285+
`goleak` workflow. `go tool golangci-lint run` is the lint
286+
gate. Its `perfsprint`, `prealloc`, and gocritic
287+
performance group are worth enabling on top of
288+
`.golangci.yml`.
289289

290290
## References
291291

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ require (
2020
github.com/vmihailenco/msgpack/v5 v5.4.1
2121
github.com/yuin/goldmark v1.8.2
2222
go.abhg.dev/goldmark/frontmatter v0.3.0
23+
golang.org/x/tools v0.43.0
2324
gopkg.in/yaml.v3 v3.0.1
2425
)
2526

@@ -269,7 +270,6 @@ require (
269270
golang.org/x/sys v0.42.0 // indirect
270271
golang.org/x/term v0.41.0 // indirect
271272
golang.org/x/text v0.35.0 // indirect
272-
golang.org/x/tools v0.43.0 // indirect
273273
google.golang.org/protobuf v1.36.8 // indirect
274274
gopkg.in/ini.v1 v1.67.0 // indirect
275275
gopkg.in/yaml.v2 v2.4.0 // indirect

0 commit comments

Comments
 (0)