Skip to content

Commit b3f414a

Browse files
author
merge-queue-bot
committed
Merge PR #828: perf: five hot-path fixes from a high-performance-go audit
2 parents 5bcd99f + edbce33 commit b3f414a

18 files changed

Lines changed: 483 additions & 61 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
settings:
3+
tokens:
4+
- process
5+
scope: section
6+
count: each
7+
max: 3
8+
diagnostics:
9+
- line: 1
10+
column: 1
11+
message: '"process" appears 8 time(s) in section (max 3)'
12+
- line: 5
13+
column: 1
14+
message: '"process" appears 4 time(s) in section (max 3)'
15+
- line: 9
16+
column: 1
17+
message: '"process" appears 4 time(s) in section (max 3)'
18+
---
19+
# A
20+
21+
process process process process.
22+
23+
## B
24+
25+
process process process process.
26+
27+
# C
28+
29+
process process process process.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
settings:
3+
scope: section
4+
max: 3
5+
min-length: 4
6+
diagnostics:
7+
- line: 1
8+
column: 1
9+
message: '"process" repeated 8 time(s) in section (max 3)'
10+
- line: 5
11+
column: 1
12+
message: '"process" repeated 4 time(s) in section (max 3)'
13+
- line: 9
14+
column: 1
15+
message: '"process" repeated 4 time(s) in section (max 3)'
16+
---
17+
# A
18+
19+
process process process process.
20+
21+
## B
22+
23+
process process process process.
24+
25+
# C
26+
27+
process process process process.

internal/rules/astutil/astutil.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,7 @@ func SectionBodies(headings []SectionHeading, paragraphs []SectionParagraph, sou
374374
for i := range headings {
375375
start := headings[i].Line
376376
end := SectionEnd(headings, i, totalLines)
377-
for lo < len(paragraphs) && paragraphs[lo].Line < start {
378-
lo++
379-
}
377+
lo = AdvancePastLine(paragraphs, lo, start)
380378
parts = parts[:0]
381379
for j := lo; j < len(paragraphs) && paragraphs[j].Line < end; j++ {
382380
parts = append(parts, paragraphs[j].ExtractText(source))
@@ -386,6 +384,35 @@ func SectionBodies(headings []SectionHeading, paragraphs []SectionParagraph, sou
386384
return bodies
387385
}
388386

387+
// AdvancePastLine returns the first index at or after lo whose entry's
388+
// Line is not less than start. paragraphs must be in ascending Line
389+
// order.
390+
//
391+
// Safe to thread lo across a sequence of calls with non-decreasing
392+
// start values over the same paragraphs slice (as a per-heading loop
393+
// over ascending headings does): a paragraph this call skips has a
394+
// Line below start, hence below every later, higher-or-equal start
395+
// too, so it can never be needed by a later call in such a sequence.
396+
//
397+
// This only advances past the skipped prefix — it does not also
398+
// advance past the paragraphs a caller goes on to collect from the
399+
// returned index onward, unlike SectionBody's binary search (which
400+
// looks up both ends of its range independently and has no cursor to
401+
// share). A hierarchical section model (see SectionEnd) lets a later,
402+
// deeper heading's window overlap an earlier, shallower one's, so
403+
// those paragraphs may need to be collected again by a later call, as
404+
// SectionBodies (which shares this helper) relies on; a caller whose
405+
// windows form a true non-overlapping partition (not SectionEnd's)
406+
// may still choose to advance its own cursor past its collected
407+
// range, since AdvancePastLine only handles the shared skip half of
408+
// that trade-off.
409+
func AdvancePastLine(paragraphs []SectionParagraph, lo, start int) int {
410+
for lo < len(paragraphs) && paragraphs[lo].Line < start {
411+
lo++
412+
}
413+
return lo
414+
}
415+
389416
// HeadingLine returns the 1-based source line of a heading node.
390417
// Setext headings expose their line via Lines(); ATX headings are found
391418
// by walking inline descendants until the first text segment. Returns 1
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package maxsectionlength
2+
3+
import (
4+
"testing"
5+
6+
"github.com/jeduden/mdsmith/internal/lint"
7+
)
8+
9+
// BenchmarkCheck_ManySections pins docs/development/high-performance-go.md's
10+
// "Skip work you don't need" guideline: countSection used to rescan the
11+
// *entire* paragraphs slice from index 0 for every heading, making
12+
// Check's per-heading loop O(headings * paragraphs) instead of
13+
// O(headings + paragraphs). This benchmark's n scales both headings and
14+
// paragraphs together, so the quadratic term dominates unless the
15+
// forward-cursor fix is in place.
16+
func BenchmarkCheck_ManySections(b *testing.B) {
17+
f, err := lint.NewFile("test.md", []byte(manySectionsDoc(600)))
18+
if err != nil {
19+
b.Fatal(err)
20+
}
21+
r := &Rule{MaxWords: 1000, MaxParagraphs: 1000}
22+
23+
b.ReportAllocs()
24+
b.ResetTimer()
25+
for i := 0; i < b.N; i++ {
26+
_ = r.Check(f)
27+
}
28+
}

internal/rules/maxsectionlength/rule.go

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,25 @@ func (r *Rule) Check(f *lint.File) []lint.Diagnostic {
106106
}
107107

108108
var diags []lint.Diagnostic
109+
// paragraphPos is a forward-only cursor into paragraphs, shared
110+
// across every heading's window. Heading windows are contiguous,
111+
// non-overlapping, and ascending (headings and paragraphs are both
112+
// in source-line order), so a paragraph consumed — or skipped as
113+
// belonging to an earlier window — by one heading can never be
114+
// needed by a later one. Threading the cursor through turns the
115+
// whole loop into a single O(headings + paragraphs) pass instead of
116+
// re-scanning all of paragraphs from index 0 per heading. See
117+
// docs/development/high-performance-go.md's "Skip work you don't
118+
// need" — the same forward-cursor pattern astutil.SectionBodies
119+
// already uses for MDS057/MDS058.
120+
paragraphPos := 0
109121
for i, h := range headings {
110122
end := totalLines
111123
if i+1 < len(headings) {
112124
end = headings[i+1].line - 1
113125
}
114126
diags = append(diags, r.checkLineLimit(f, h, end)...)
115-
diags = append(diags, r.checkWordAndParagraphLimits(f, h, end, paragraphs)...)
127+
diags = append(diags, r.checkWordAndParagraphLimits(f, h, end, paragraphs, &paragraphPos)...)
116128
}
117129
return diags
118130
}
@@ -138,12 +150,12 @@ func (r *Rule) checkLineLimit(f *lint.File, h heading, end int) []lint.Diagnosti
138150
}
139151

140152
func (r *Rule) checkWordAndParagraphLimits(
141-
f *lint.File, h heading, end int, paragraphs []paragraph,
153+
f *lint.File, h heading, end int, paragraphs []paragraph, paragraphPos *int,
142154
) []lint.Diagnostic {
143155
if r.MaxWords <= 0 && r.MinWords <= 0 && r.MaxParagraphs <= 0 {
144156
return nil
145157
}
146-
words, paraCount := countSection(paragraphs, h.line, end)
158+
words, paraCount := countSection(paragraphs, paragraphPos, h.line, end)
147159
var diags []lint.Diagnostic
148160
if r.MaxWords > 0 && words > r.MaxWords {
149161
diags = append(diags, lint.Diagnostic{
@@ -347,16 +359,27 @@ func collectParagraphs(f *lint.File) []paragraph {
347359
return out
348360
}
349361

350-
// countSection sums words and paragraphs for paragraphs whose start
351-
// line falls within [start, end].
352-
func countSection(paragraphs []paragraph, start, end int) (words, count int) {
353-
for _, p := range paragraphs {
354-
if p.line < start || p.line > end {
355-
continue
356-
}
357-
words += p.words
362+
// countSection sums the words and paragraph count of every entry in
363+
// paragraphs whose line falls in [start, end]. paragraphs must be in
364+
// ascending line order (astutil.CollectSectionParagraphs guarantees
365+
// this). *pos is a forward-only cursor: callers processing a sequence
366+
// of non-overlapping, ascending [start, end] windows over the same
367+
// paragraphs slice (as Check's per-heading loop does) should thread
368+
// the same pos through every call, since a paragraph this call skips
369+
// or consumes can never belong to a later, higher-numbered window —
370+
// letting the whole sequence of calls run in O(len(paragraphs)) total
371+
// instead of O(len(headings) * len(paragraphs)).
372+
func countSection(paragraphs []paragraph, pos *int, start, end int) (words, count int) {
373+
for *pos < len(paragraphs) && paragraphs[*pos].line < start {
374+
*pos++
375+
}
376+
i := *pos
377+
for i < len(paragraphs) && paragraphs[i].line <= end {
378+
words += paragraphs[i].words
358379
count++
380+
i++
359381
}
382+
*pos = i
360383
return words, count
361384
}
362385

internal/rules/maxsectionlength/rule_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,28 @@ func mustFile(t *testing.T, src string) *lint.File {
2020
return f
2121
}
2222

23+
// TestCountSection_SkipsPrecedingParagraphs exercises countSection's
24+
// pos-advance loop directly: a paragraph strictly before the window's
25+
// start must be skipped (not counted), and the cursor must carry
26+
// forward correctly into a second call for a later, higher window —
27+
// the shape Check's per-heading loop drives it with.
28+
func TestCountSection_SkipsPrecedingParagraphs(t *testing.T) {
29+
paragraphs := []paragraph{
30+
{line: 2, words: 3}, // preceding paragraph, must be skipped for [5, 10]
31+
{line: 6, words: 4},
32+
{line: 12, words: 5},
33+
}
34+
var pos int
35+
36+
words, count := countSection(paragraphs, &pos, 5, 10)
37+
assert.Equal(t, 4, words, "only the line-6 paragraph falls in [5, 10]")
38+
assert.Equal(t, 1, count)
39+
40+
words, count = countSection(paragraphs, &pos, 11, 20)
41+
assert.Equal(t, 5, words, "cursor must still reach the line-12 paragraph")
42+
assert.Equal(t, 1, count)
43+
}
44+
2345
func TestCheck_NoHeadings_NoDiagnostic(t *testing.T) {
2446
f := mustFile(t, "just some text\nand more\n")
2547
r := &Rule{Max: 5}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package noundefinedreferencelabels
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
// proseWithoutBrackets builds n lines of ordinary prose containing no
9+
// '[' at all — the worst case for nextBracket's forward scan, since
10+
// every byte in the file must be inspected before the scanner can
11+
// report EOF.
12+
func proseWithoutBrackets(lines int) []byte {
13+
var b strings.Builder
14+
for i := 0; i < lines; i++ {
15+
b.WriteString("The quick brown fox jumps over the lazy dog again and again.\n")
16+
}
17+
return []byte(b.String())
18+
}
19+
20+
// BenchmarkNextBracket_NoBrackets pins the doc's "bytes.IndexByte over
21+
// a hand-rolled byte loop" guideline
22+
// (docs/development/high-performance-go.md#strings-and-bytes):
23+
// collectBrackets calls nextBracket to scan the *entire* file source
24+
// once, byte by byte, looking for '['. On a file with no reference-style
25+
// links at all (the common case — most files don't use them) the loop
26+
// must inspect every byte before reporting EOF, so this is exactly the
27+
// "big Source scan" case the guideline calls out as worth vectorizing.
28+
func BenchmarkNextBracket_NoBrackets(b *testing.B) {
29+
source := proseWithoutBrackets(200)
30+
b.ReportAllocs()
31+
for i := 0; i < b.N; i++ {
32+
_, _, _, _, ok := nextBracket(source, 0)
33+
if ok {
34+
b.Fatal("expected no bracket to be found")
35+
}
36+
}
37+
}

internal/rules/noundefinedreferencelabels/rule.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,12 @@ func advanceBracket(brs []bracket, i, pos int) int {
245245
// task 7. Empty contents (`[]`) return contentStart == contentEnd;
246246
// the caller decides whether that is valid for the pattern.
247247
func nextBracket(source []byte, pos int) (open, contentStart, contentEnd, closeAfter int, ok bool) {
248-
for pos < len(source) {
249-
if source[pos] != '[' {
250-
pos++
251-
continue
248+
for {
249+
idx := bytes.IndexByte(source[pos:], '[')
250+
if idx < 0 {
251+
return 0, 0, 0, 0, false
252252
}
253+
pos += idx
253254
open = pos
254255
contentStart = pos + 1
255256
i := contentStart
@@ -263,7 +264,6 @@ func nextBracket(source []byte, pos int) (open, contentStart, contentEnd, closeA
263264
// advance past the orphan `[` and keep scanning.
264265
pos++
265266
}
266-
return 0, 0, 0, 0, false
267267
}
268268

269269
// scanFullRefs walks source for `[text][label]` patterns. The byte
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package occurrence
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/jeduden/mdsmith/internal/lint"
8+
)
9+
10+
// manySectionsDoc builds a document with n level-2 headings, each
11+
// followed by one short paragraph — the shape that drives
12+
// checkSections's per-heading loop and, through it, the paragraph
13+
// range scans.
14+
func manySectionsDoc(n int) string {
15+
var b strings.Builder
16+
for i := 0; i < n; i++ {
17+
b.WriteString("## Section\n\nA short paragraph with a foo token in it.\n\n")
18+
}
19+
return b.String()
20+
}
21+
22+
// BenchmarkCheck_ManySections pins docs/development/high-performance-go.md's
23+
// "Skip work you don't need" guideline: checkSections's range helpers used
24+
// to rescan the *entire* paragraphs slice from index 0 for every heading,
25+
// making the section-scope Check O(headings * paragraphs) instead of
26+
// O(headings + paragraphs).
27+
func BenchmarkCheck_ManySections(b *testing.B) {
28+
f, err := lint.NewFile("test.md", []byte(manySectionsDoc(600)))
29+
if err != nil {
30+
b.Fatal(err)
31+
}
32+
r := &Rule{Scope: "section", Count: "each", Tokens: []string{"foo"}, CaseSensitive: true}
33+
34+
b.ReportAllocs()
35+
b.ResetTimer()
36+
for i := 0; i < b.N; i++ {
37+
_ = r.Check(f)
38+
}
39+
}

0 commit comments

Comments
 (0)