Merge pull request #789 from peco/fix-merge-matches-alloc

fix: in-place mergeMatches to avoid per-merge heap allocation

filter/bench_test.go

@@ -77,3 +77,33 @@ func BenchmarkRegexpFilter(b *testing.B) {
 		cancel()
 	}
 }
+
+// BenchmarkRegexpFilterOverlapping benchmarks the regexp filter with a query
+// that produces overlapping match ranges, exercising the mergeMatches path.
+// Each line contains repeating "aabb" patterns; the query terms "aab" and "abb"
+// produce match ranges that overlap (e.g. [0,3] and [1,4]), forcing mergeMatches
+// to be called on every line. With 10k lines the aggregate allocation difference
+// from in-place vs make([]int,2) becomes measurable.
+func BenchmarkRegexpFilterOverlapping(b *testing.B) {
+	// Build a line with many "aabb" repeats so that "aab" and "abb" each match
+	// many times with overlapping ranges between the two terms.
+	base := strings.Repeat("aabb", 20) // 80 chars
+	lines := make([]line.Line, 10_000)
+	for i := range lines {
+		lines[i] = line.NewRaw(uint64(i), base, false, false)
+	}
+
+	f := NewIgnoreCase()
+	// "aab" and "abb" share the middle "ab" in each "aabb" group, so their
+	// match ranges overlap after sorting by start position.
+	query := "aab abb"
+
+	b.ResetTimer()
+	b.ReportAllocs()
+	for b.Loop() {
+		ctx, cancel := context.WithTimeout(f.NewContext(context.Background(), query), 10*time.Second)
+		ch := make(chan line.Line, len(lines))
+		_ = f.Apply(ctx, lines, pipeline.ChanOutput(ch))
+		cancel()
+	}
+}

filter/filter.go

@@ -93,15 +93,9 @@ func matchOverlaps(a []int, b []int) bool {
 }
 
 // mergeMatches combines two overlapping match ranges into a single range
-// spanning both.
+// spanning both. It mutates and returns a to avoid a heap allocation.
 func mergeMatches(a []int, b []int) []int {
-	ret := make([]int, 2)
-
-	// Note: In practice this should never happen
-	// because we're sorting by N[0] before calling
-	// this routine, but for completeness' sake...
-	ret[0] = min(a[0], b[0])
-
-	ret[1] = max(a[1], b[1])
-	return ret
+	a[0] = min(a[0], b[0])
+	a[1] = max(a[1], b[1])
+	return a
 }

filter/filter_test.go

@@ -634,6 +634,31 @@ func testFuzzyMatch(octx context.Context, t *testing.T, filter Filter) {
 	}
 }
 
+func TestMergeMatches(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name string
+		a    []int
+		b    []int
+		want []int
+	}{
+		{"a before b overlapping", []int{1, 5}, []int{3, 7}, []int{1, 7}},
+		{"b before a overlapping", []int{3, 7}, []int{1, 5}, []int{1, 7}},
+		{"identical ranges", []int{2, 4}, []int{2, 4}, []int{2, 4}},
+		{"a contains b", []int{0, 10}, []int{3, 7}, []int{0, 10}},
+		{"adjacent ranges", []int{0, 3}, []int{3, 6}, []int{0, 6}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := mergeMatches(tt.a, tt.b)
+			require.Equal(t, tt.want, got)
+			// Verify in-place mutation: got should be the same slice as a
+			require.Same(t, &tt.a[0], &got[0], "mergeMatches should mutate a in place")
+		})
+	}
+}
+
 // TestMatchAcrossANSIColorBoundary verifies that filter queries match
 // against the ANSI-stripped text, so a pattern spanning characters
 // rendered in different colors still produces a match.