perf: place alerts by offset instead of searching for their text

Signed-off-by: Joseph Kato <joseph@jdkato.io>

Author: jdkato

cmd/vale/main.go

@@ -83,13 +83,18 @@ func handleError(err error) {
 }
 
 func main() {
+	// Every exit path below calls this explicitly: os.Exit skips deferred
+	// functions, so a defer here would silently drop the profile.
+	stopProfiling := startProfiling()
+
 	pflag.Parse()
 
 	args := pflag.Args()
 	argc := len(args)
 
 	if Flags.Version { //nolint:gocritic
 		fmt.Println("vale version " + version)
+		stopProfiling()
 		os.Exit(0)
 	} else if Flags.Help {
 		pflag.Usage()
@@ -103,6 +108,7 @@ func main() {
 			if err := cmd(args[1:], &Flags); err != nil {
 				handleError(err)
 			}
+			stopProfiling()
 			os.Exit(0)
 		}
 	}
@@ -125,9 +131,11 @@ func main() {
 	hasErrors, err := PrintAlerts(linted, config)
 	if err != nil {
 		handleError(err)
-	} else if hasErrors && !Flags.NoExit {
-		os.Exit(1)
 	}
 
+	stopProfiling()
+	if hasErrors && !Flags.NoExit {
+		os.Exit(1)
+	}
 	os.Exit(0)
 }

cmd/vale/profile.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"runtime"
+	"runtime/pprof"
+)
+
+// Profiling is written when VALE_CPUPROFILE or VALE_MEMPROFILE names a file.
+//
+// Environment variables rather than flags: Vale's flag set is user-facing and
+// stable, while this is a development tool. It also means a profile can be
+// captured from a run driven by an editor or a script without changing the
+// command line.
+type profiler struct {
+	cpu *os.File
+	mem string
+}
+
+// startProfiling begins any profiling the environment asks for. The returned
+// function must be called before the process exits.
+func startProfiling() func() {
+	p := &profiler{mem: os.Getenv("VALE_MEMPROFILE")}
+
+	if path := os.Getenv("VALE_CPUPROFILE"); path != "" {
+		f, err := os.Create(path)
+		switch {
+		case err != nil:
+			fmt.Fprintf(os.Stderr, "vale: creating CPU profile: %v\n", err)
+		default:
+			if serr := pprof.StartCPUProfile(f); serr != nil {
+				fmt.Fprintf(os.Stderr, "vale: starting CPU profile: %v\n", serr)
+				f.Close()
+			} else {
+				p.cpu = f
+			}
+		}
+	}
+
+	return p.stop
+}
+
+func (p *profiler) stop() {
+	if p.cpu != nil {
+		pprof.StopCPUProfile()
+		p.cpu.Close()
+	}
+
+	if p.mem == "" {
+		return
+	}
+
+	f, err := os.Create(p.mem)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "vale: creating memory profile: %v\n", err)
+		return
+	}
+	defer f.Close()
+
+	// A profile of the live heap, so the numbers describe what Vale is holding
+	// rather than everything it ever allocated.
+	runtime.GC()
+	if werr := pprof.WriteHeapProfile(f); werr != nil {
+		fmt.Fprintf(os.Stderr, "vale: writing memory profile: %v\n", werr)
+	}
+}

internal/check/anchor.go

@@ -0,0 +1,87 @@
+package check
+
+import (
+	"unicode/utf8"
+
+	"github.com/errata-ai/vale/v3/internal/core"
+	"github.com/errata-ai/vale/v3/internal/nlp"
+)
+
+// anchor converts an alert's block-relative rune span into an absolute byte
+// span within the document.
+//
+// Vale otherwise locates an alert by searching the document for the text it
+// matched, and masking that text so the next alert from the same rule finds a
+// later occurrence. That copies the whole context per alert, which is the bulk
+// of Vale's allocation on a document with many findings -- and it mislocates a
+// match that appears more than once or that spans irregular whitespace.
+//
+// An anchored alert needs none of that: it already says exactly where it is.
+//
+// Anchoring is skipped unless the block's position is known *and* verifiable.
+// Blocks carved out of markup hold text that has been stripped of its markup,
+// so their offsets do not address the original document; checking that the
+// text really sits where the offset claims is what keeps those out.
+func anchor(a *core.Alert, blk nlp.Block) {
+	if blk.Offset < 0 || len(a.Span) != 2 {
+		return
+	}
+
+	// The offset is only ever set after a successful search for this text, so
+	// it is already known good. It cannot be re-checked here: extraction masks
+	// text in the context buffer as it goes, so by now the bytes at that
+	// position may have been overwritten -- length-preservingly, which is what
+	// keeps the offset itself valid.
+	if blk.Offset+len(blk.Text) > len(blk.Context) {
+		return
+	}
+
+	lo, hi, ok := runeSpanToBytes(blk.Text, a.Span[0], a.Span[1])
+	if !ok {
+		return
+	}
+
+	a.Span = []int{blk.Offset + lo, blk.Offset + hi}
+	a.HasByteOffsets = true
+}
+
+// runeSpanToBytes converts a rune-indexed span into byte offsets.
+//
+// regexp2 reports positions in runes; everything downstream of the check
+// addresses the document in bytes.
+func runeSpanToBytes(s string, from, to int) (int, int, bool) {
+	if from < 0 || to < from {
+		return 0, 0, false
+	}
+
+	var (
+		runes  int
+		lo, hi = -1, -1
+		i      int
+	)
+	for i = 0; i < len(s); {
+		if runes == from && lo < 0 {
+			lo = i
+		}
+		if runes == to {
+			hi = i
+			break
+		}
+		_, size := utf8.DecodeRuneInString(s[i:])
+		i += size
+		runes++
+	}
+
+	// A span reaching the end of the string ends past the final rune.
+	if runes == from && lo < 0 {
+		lo = i
+	}
+	if runes == to && hi < 0 {
+		hi = i
+	}
+
+	if lo < 0 || hi < 0 || hi < lo {
+		return 0, 0, false
+	}
+	return lo, hi, true
+}

internal/check/anchor_test.go

@@ -0,0 +1,69 @@
+package check
+
+import "testing"
+
+// runeSpanToBytes underpins every anchored alert; an off-by-one here
+// mislocates findings rather than failing loudly.
+func TestRuneSpanToBytes(t *testing.T) {
+	cases := []struct {
+		name     string
+		s        string
+		from, to int
+		want     string
+		ok       bool
+	}{
+		{"ascii start", "hello world", 0, 5, "hello", true},
+		{"ascii middle", "hello world", 6, 11, "world", true},
+		{"ascii to end", "hello", 0, 5, "hello", true},
+		{"empty span", "hello", 2, 2, "", true},
+		{"multibyte before", "héllo wörld", 6, 11, "wörld", true},
+		{"multibyte inside", "héllo", 0, 5, "héllo", true},
+		{"emoji", "a 🎉 b", 2, 3, "🎉", true},
+		{"cjk", "漢字テスト", 0, 2, "漢字", true},
+		{"whole string", "naïve", 0, 5, "naïve", true},
+		{"negative", "abc", -1, 2, "", false},
+		{"reversed", "abc", 2, 1, "", false},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			lo, hi, ok := runeSpanToBytes(c.s, c.from, c.to)
+			if ok != c.ok {
+				t.Fatalf("ok = %v, want %v", ok, c.ok)
+			}
+			if !ok {
+				return
+			}
+			if got := c.s[lo:hi]; got != c.want {
+				t.Errorf("s[%d:%d] = %q, want %q", lo, hi, got, c.want)
+			}
+		})
+	}
+}
+
+// The conversion must agree with the rune slicing it replaces, for every span
+// of a multi-byte string.
+func TestRuneSpanToBytesMatchesRuneSlicing(t *testing.T) {
+	for _, s := range []string{
+		"hello world",
+		"héllo wörld",
+		"漢字テストです",
+		"a 🎉 b 😀 c",
+		"naïve café résumé",
+		"",
+	} {
+		runes := []rune(s)
+		for from := 0; from <= len(runes); from++ {
+			for to := from; to <= len(runes); to++ {
+				lo, hi, ok := runeSpanToBytes(s, from, to)
+				if !ok {
+					t.Fatalf("%q [%d:%d]: not ok", s, from, to)
+				}
+				want := string(runes[from:to])
+				if got := s[lo:hi]; got != want {
+					t.Errorf("%q [%d:%d] = %q, want %q", s, from, to, got, want)
+				}
+			}
+		}
+	}
+}

internal/check/capitalization.go

@@ -135,6 +135,7 @@ func (c Capitalization) Run(blk nlp.Block, _ *core.File, cfg *core.Config) ([]co
 			c.Description, blk.Text, expected)
 		a.Action = action
 
+		anchor(&a, blk)
 		alerts = append(alerts, a)
 	}
 

internal/check/consistency.go

@@ -100,6 +100,13 @@ func (o Consistency) Run(blk nlp.Block, f *core.File, cfg *core.Config) ([]core.
 		if matches != nil && core.AllStringsInSlice(s.subs, f.Sequences) {
 			o.Name = o.Extends
 
+			// Not anchored, deliberately. `loc` is whatever the submatch loop
+			// above left behind, which is the *last* match in the block rather
+			// than the one being reported; searching for the matched text
+			// instead lands on the first occurrence, which is what this check
+			// has always reported. Anchoring would promote that leftover into
+			// the output. The rule fires at most once per block, so there is
+			// nothing to gain by it either.
 			a, err := makeAlert(o.Definition, loc, txt, cfg)
 			if err != nil {
 				return alerts, err

internal/check/existence.go

@@ -97,6 +97,7 @@ func (e Existence) Run(blk nlp.Block, _ *core.File, cfg *core.Config) ([]core.Al
 			if erra != nil {
 				return alerts, erra
 			}
+			anchor(&a, blk)
 			alerts = append(alerts, a)
 		}
 	}

internal/check/occurrence.go

@@ -102,6 +102,9 @@ func (o Occurrence) Run(blk nlp.Block, _ *core.File, cfg *core.Config) ([]core.A
 			if err != nil {
 				return alerts, err
 			}
+			// Only this branch: the zero-occurrence case above reports a line
+			// number, not a span into the text.
+			anchor(&a, blk)
 		}
 
 		// Pass the count as an int (not a string) so messages can use either

internal/check/repetition.go

@@ -113,6 +113,8 @@ func (o Repetition) Run(blk nlp.Block, _ *core.File, cfg *core.Config) ([]core.A
 
 					a.Message, a.Description = formatMessages(o.Message,
 						o.Description, curr)
+
+					anchor(&a, blk)
 					alerts = append(alerts, a)
 					count = 0
 				}

internal/check/substitution.go

@@ -172,6 +172,7 @@ func (s Substitution) Run(blk nlp.Block, _ *core.File, cfg *core.Config) ([]core
 						s.Description, expected, observed)
 					a.Action = action
 
+					anchor(&a, blk)
 					alerts = append(alerts, a)
 				}
 			}