perf(copy): avoid history row copies during search
- commit
- da79df9f66f6c8c8fd79800a3b7fbebd80aef848
- author
- brice · · 2026-07-12 17:39 UTC
- parent
- 7cf3f77
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +71 -10
M internal/usecase/copy/mode.go +24 -3
| @@ -65,6 +65,27 @@ func (s Snapshot) Row(i int) []renderer.Cell { | |||
| 65 | 65 | return s.screen.Row(i) | |
| 66 | 66 | } | |
| 67 | 67 | ||
| 68 | + | func (s Snapshot) rangeRows(yield func(int, []renderer.Cell) bool) { | |
| 69 | + | rowIndex := 0 | |
| 70 | + | stopped := false | |
| 71 | + | s.history.Range(func(row []renderer.Cell) bool { | |
| 72 | + | if !yield(rowIndex, row) { | |
| 73 | + | stopped = true | |
| 74 | + | return false | |
| 75 | + | } | |
| 76 | + | rowIndex++ | |
| 77 | + | return true | |
| 78 | + | }) | |
| 79 | + | if stopped { | |
| 80 | + | return | |
| 81 | + | } | |
| 82 | + | for y := range s.screen.Height { | |
| 83 | + | if !yield(rowIndex+y, s.screen.Row(y)) { | |
| 84 | + | return | |
| 85 | + | } | |
| 86 | + | } | |
| 87 | + | } | |
| 88 | + | ||
| 68 | 89 | // Mode stores per-client scrollback viewport and line-selection state. | |
| 69 | 90 | type SearchMatch struct { | |
| 70 | 91 | Row int | |
| @@ -103,8 +124,7 @@ func FindMatches(s Snapshot, query string) []SearchMatch { | |||
| 103 | 124 | } | |
| 104 | 125 | needle := lowerRunes(query) | |
| 105 | 126 | matches := make([]SearchMatch, 0) | |
| 106 | - | for row := range s.Len() { | |
| 107 | - | cells := s.Row(row) | |
| 127 | + | s.rangeRows(func(row int, cells []renderer.Cell) bool { | |
| 108 | 128 | haystack, cellIndexes := searchableCells(cells) | |
| 109 | 129 | var text string | |
| 110 | 130 | for start := 0; start+len(needle) <= len(haystack); { | |
| @@ -123,7 +143,8 @@ func FindMatches(s Snapshot, query string) []SearchMatch { | |||
| 123 | 143 | matches = append(matches, SearchMatch{Row: row, Start: cellIndexes[start], End: cellEnd, Text: text}) | |
| 124 | 144 | start += len(needle) | |
| 125 | 145 | } | |
| 126 | - | } | |
| 146 | + | return true | |
| 147 | + | }) | |
| 127 | 148 | return matches | |
| 128 | 149 | } | |
| 129 | 150 | ||
| @@ -65,6 +65,27 @@ func (s Snapshot) Row(i int) []renderer.Cell { | |||||
| 65 | return s.screen.Row(i) | 65 | return s.screen.Row(i) | ||
| 66 | } | 66 | } | ||
| 67 | 67 | ||||
| 68 | + | func (s Snapshot) rangeRows(yield func(int, []renderer.Cell) bool) { | |||
| 69 | + | rowIndex := 0 | |||
| 70 | + | stopped := false | |||
| 71 | + | s.history.Range(func(row []renderer.Cell) bool { | |||
| 72 | + | if !yield(rowIndex, row) { | |||
| 73 | + | stopped = true | |||
| 74 | + | return false | |||
| 75 | + | } | |||
| 76 | + | rowIndex++ | |||
| 77 | + | return true | |||
| 78 | + | }) | |||
| 79 | + | if stopped { | |||
| 80 | + | return | |||
| 81 | + | } | |||
| 82 | + | for y := range s.screen.Height { | |||
| 83 | + | if !yield(rowIndex+y, s.screen.Row(y)) { | |||
| 84 | + | return | |||
| 85 | + | } | |||
| 86 | + | } | |||
| 87 | + | } | |||
| 88 | + | ||||
| 68 | // Mode stores per-client scrollback viewport and line-selection state. | 89 | // Mode stores per-client scrollback viewport and line-selection state. | ||
| 69 | type SearchMatch struct { | 90 | type SearchMatch struct { | ||
| 70 | Row int | 91 | Row int | ||
| @@ -103,8 +124,7 @@ func FindMatches(s Snapshot, query string) []SearchMatch { | |||||
| 103 | } | 124 | } | ||
| 104 | needle := lowerRunes(query) | 125 | needle := lowerRunes(query) | ||
| 105 | matches := make([]SearchMatch, 0) | 126 | matches := make([]SearchMatch, 0) | ||
| 106 | - | for row := range s.Len() { | 127 | + | s.rangeRows(func(row int, cells []renderer.Cell) bool { |
| 107 | - | cells := s.Row(row) | |||
| 108 | haystack, cellIndexes := searchableCells(cells) | 128 | haystack, cellIndexes := searchableCells(cells) | ||
| 109 | var text string | 129 | var text string | ||
| 110 | for start := 0; start+len(needle) <= len(haystack); { | 130 | for start := 0; start+len(needle) <= len(haystack); { | ||
| @@ -123,7 +143,8 @@ func FindMatches(s Snapshot, query string) []SearchMatch { | |||||
| 123 | matches = append(matches, SearchMatch{Row: row, Start: cellIndexes[start], End: cellEnd, Text: text}) | 143 | matches = append(matches, SearchMatch{Row: row, Start: cellIndexes[start], End: cellEnd, Text: text}) | ||
| 124 | start += len(needle) | 144 | start += len(needle) | ||
| 125 | } | 145 | } | ||
| 126 | - | } | 146 | + | return true |
| 147 | + | }) | |||
| 127 | return matches | 148 | return matches | ||
| 128 | } | 149 | } | ||
| 129 | 150 | ||||
M internal/usecase/copy/mode_test.go +18 -0
| @@ -374,6 +374,24 @@ func TestFindMatchesKeepsRuneAndCellIndexesAlignedWhenLowercasing(t *testing.T) | |||
| 374 | 374 | require.Equal(t, 2, matches[0].End) | |
| 375 | 375 | } | |
| 376 | 376 | ||
| 377 | + | func TestFindMatchesDoesNotCopyEveryHistoryRow(t *testing.T) { | |
| 378 | + | const rows = 100 | |
| 379 | + | history := vt.NewHistory(vt.HistoryConfig{MaxRows: rows, ChunkRows: 10}) | |
| 380 | + | for range rows { | |
| 381 | + | history.Append(row(strings.Repeat("x", 120))) | |
| 382 | + | } | |
| 383 | + | s := NewSnapshot(history, renderer.NewFrame(120, 1)) | |
| 384 | + | ||
| 385 | + | allocs := testing.AllocsPerRun(10, func() { | |
| 386 | + | if matches := FindMatches(s, "missing"); len(matches) != 0 { | |
| 387 | + | t.Fatalf("unexpected matches: %d", len(matches)) | |
| 388 | + | } | |
| 389 | + | }) | |
| 390 | + | if allocs >= 250 { | |
| 391 | + | t.Fatalf("FindMatches allocations = %.0f, want < 250 (no row copy per history row)", allocs) | |
| 392 | + | } | |
| 393 | + | } | |
| 394 | + | ||
| 377 | 395 | func TestCopyModeSearchUsesDisplayCellOffsetsForWideRows(t *testing.T) { | |
| 378 | 396 | row := []renderer.Cell{ | |
| 379 | 397 | {Rune: '界'}, | |
| @@ -374,6 +374,24 @@ func TestFindMatchesKeepsRuneAndCellIndexesAlignedWhenLowercasing(t *testing.T) | |||||
| 374 | require.Equal(t, 2, matches[0].End) | 374 | require.Equal(t, 2, matches[0].End) | ||
| 375 | } | 375 | } | ||
| 376 | 376 | ||||
| 377 | + | func TestFindMatchesDoesNotCopyEveryHistoryRow(t *testing.T) { | |||
| 378 | + | const rows = 100 | |||
| 379 | + | history := vt.NewHistory(vt.HistoryConfig{MaxRows: rows, ChunkRows: 10}) | |||
| 380 | + | for range rows { | |||
| 381 | + | history.Append(row(strings.Repeat("x", 120))) | |||
| 382 | + | } | |||
| 383 | + | s := NewSnapshot(history, renderer.NewFrame(120, 1)) | |||
| 384 | + | ||||
| 385 | + | allocs := testing.AllocsPerRun(10, func() { | |||
| 386 | + | if matches := FindMatches(s, "missing"); len(matches) != 0 { | |||
| 387 | + | t.Fatalf("unexpected matches: %d", len(matches)) | |||
| 388 | + | } | |||
| 389 | + | }) | |||
| 390 | + | if allocs >= 250 { | |||
| 391 | + | t.Fatalf("FindMatches allocations = %.0f, want < 250 (no row copy per history row)", allocs) | |||
| 392 | + | } | |||
| 393 | + | } | |||
| 394 | + | ||||
| 377 | func TestCopyModeSearchUsesDisplayCellOffsetsForWideRows(t *testing.T) { | 395 | func TestCopyModeSearchUsesDisplayCellOffsetsForWideRows(t *testing.T) { | ||
| 378 | row := []renderer.Cell{ | 396 | row := []renderer.Cell{ | ||
| 379 | {Rune: '界'}, | 397 | {Rune: '界'}, | ||
M internal/usecase/daemon/performance_benchmark_test.go +17 -7
| @@ -298,24 +298,34 @@ func benchmarkDaemonLargeHistory(b *testing.B, workload string, run func(*perfor | |||
| 298 | 298 | func benchmarkDaemonCopyOperation(b *testing.B, fixture *performanceFixture, workload string, run func(*performanceFixture)) { | |
| 299 | 299 | b.Helper() | |
| 300 | 300 | b.ReportAllocs() | |
| 301 | + | if workload == "copy-search" { | |
| 302 | + | fixture.d.enterCopyMode(fixture.sess, fixture.ac) | |
| 303 | + | fixture.ac.ackOutputState(fixture.ac.output.next) | |
| 304 | + | b.ResetTimer() | |
| 305 | + | for range b.N { | |
| 306 | + | run(fixture) | |
| 307 | + | fixture.ac.ackOutputState(fixture.ac.output.next) | |
| 308 | + | } | |
| 309 | + | b.StopTimer() | |
| 310 | + | if fixture.searchMatches() == 0 { | |
| 311 | + | b.Fatal("copy search produced no deterministic matches") | |
| 312 | + | } | |
| 313 | + | b.ReportMetric(1, "copyoperations/op") | |
| 314 | + | return | |
| 315 | + | } | |
| 316 | + | ||
| 301 | 317 | b.ResetTimer() | |
| 302 | 318 | operations := 0 | |
| 303 | 319 | for range b.N { | |
| 304 | 320 | b.StopTimer() | |
| 305 | 321 | fixture.d.exitCopyMode(fixture.ac) | |
| 306 | - | if workload == "copy-search" { | |
| 307 | - | fixture.d.enterCopyMode(fixture.sess, fixture.ac) | |
| 308 | - | } | |
| 309 | 322 | fixture.ac.ackOutputState(fixture.ac.output.next) | |
| 310 | 323 | b.StartTimer() | |
| 311 | 324 | run(fixture) | |
| 312 | 325 | b.StopTimer() | |
| 313 | - | if workload == "copy-enter" && !fixture.copyModeActive() { | |
| 326 | + | if !fixture.copyModeActive() { | |
| 314 | 327 | b.Fatal("copy enter did not install a copy mode") | |
| 315 | 328 | } | |
| 316 | - | if workload == "copy-search" && fixture.searchMatches() == 0 { | |
| 317 | - | b.Fatal("copy search produced no deterministic matches") | |
| 318 | - | } | |
| 319 | 329 | operations++ | |
| 320 | 330 | } | |
| 321 | 331 | if operations != b.N { | |
| @@ -298,24 +298,34 @@ func benchmarkDaemonLargeHistory(b *testing.B, workload string, run func(*perfor | |||||
| 298 | func benchmarkDaemonCopyOperation(b *testing.B, fixture *performanceFixture, workload string, run func(*performanceFixture)) { | 298 | func benchmarkDaemonCopyOperation(b *testing.B, fixture *performanceFixture, workload string, run func(*performanceFixture)) { | ||
| 299 | b.Helper() | 299 | b.Helper() | ||
| 300 | b.ReportAllocs() | 300 | b.ReportAllocs() | ||
| 301 | + | if workload == "copy-search" { | |||
| 302 | + | fixture.d.enterCopyMode(fixture.sess, fixture.ac) | |||
| 303 | + | fixture.ac.ackOutputState(fixture.ac.output.next) | |||
| 304 | + | b.ResetTimer() | |||
| 305 | + | for range b.N { | |||
| 306 | + | run(fixture) | |||
| 307 | + | fixture.ac.ackOutputState(fixture.ac.output.next) | |||
| 308 | + | } | |||
| 309 | + | b.StopTimer() | |||
| 310 | + | if fixture.searchMatches() == 0 { | |||
| 311 | + | b.Fatal("copy search produced no deterministic matches") | |||
| 312 | + | } | |||
| 313 | + | b.ReportMetric(1, "copyoperations/op") | |||
| 314 | + | return | |||
| 315 | + | } | |||
| 316 | + | ||||
| 301 | b.ResetTimer() | 317 | b.ResetTimer() | ||
| 302 | operations := 0 | 318 | operations := 0 | ||
| 303 | for range b.N { | 319 | for range b.N { | ||
| 304 | b.StopTimer() | 320 | b.StopTimer() | ||
| 305 | fixture.d.exitCopyMode(fixture.ac) | 321 | fixture.d.exitCopyMode(fixture.ac) | ||
| 306 | - | if workload == "copy-search" { | |||
| 307 | - | fixture.d.enterCopyMode(fixture.sess, fixture.ac) | |||
| 308 | - | } | |||
| 309 | fixture.ac.ackOutputState(fixture.ac.output.next) | 322 | fixture.ac.ackOutputState(fixture.ac.output.next) | ||
| 310 | b.StartTimer() | 323 | b.StartTimer() | ||
| 311 | run(fixture) | 324 | run(fixture) | ||
| 312 | b.StopTimer() | 325 | b.StopTimer() | ||
| 313 | - | if workload == "copy-enter" && !fixture.copyModeActive() { | 326 | + | if !fixture.copyModeActive() { |
| 314 | b.Fatal("copy enter did not install a copy mode") | 327 | b.Fatal("copy enter did not install a copy mode") | ||
| 315 | } | 328 | } | ||
| 316 | - | if workload == "copy-search" && fixture.searchMatches() == 0 { | |||
| 317 | - | b.Fatal("copy search produced no deterministic matches") | |||
| 318 | - | } | |||
| 319 | operations++ | 329 | operations++ | ||
| 320 | } | 330 | } | ||
| 321 | if operations != b.N { | 331 | if operations != b.N { | ||
M pkg/vt/history.go +12 -0
| @@ -129,6 +129,18 @@ func (v HistoryView) Chunk(i int) *HistoryChunk { | |||
| 129 | 129 | return v.chunks[i] | |
| 130 | 130 | } | |
| 131 | 131 | ||
| 132 | + | // Range calls yield for each row in oldest-first order. The row is borrowed | |
| 133 | + | // immutable storage: yield must not mutate or retain it after returning. | |
| 134 | + | func (v HistoryView) Range(yield func([]renderer.Cell) bool) { | |
| 135 | + | for _, chunk := range v.chunks { | |
| 136 | + | for _, row := range chunk.rows { | |
| 137 | + | if !yield(row) { | |
| 138 | + | return | |
| 139 | + | } | |
| 140 | + | } | |
| 141 | + | } | |
| 142 | + | } | |
| 143 | + | ||
| 132 | 144 | // Row returns a copy of the row at i, or nil when i is out of range. | |
| 133 | 145 | func (v HistoryView) Row(i int) []renderer.Cell { | |
| 134 | 146 | if i < 0 { | |
| @@ -129,6 +129,18 @@ func (v HistoryView) Chunk(i int) *HistoryChunk { | |||||
| 129 | return v.chunks[i] | 129 | return v.chunks[i] | ||
| 130 | } | 130 | } | ||
| 131 | 131 | ||||
| 132 | + | // Range calls yield for each row in oldest-first order. The row is borrowed | |||
| 133 | + | // immutable storage: yield must not mutate or retain it after returning. | |||
| 134 | + | func (v HistoryView) Range(yield func([]renderer.Cell) bool) { | |||
| 135 | + | for _, chunk := range v.chunks { | |||
| 136 | + | for _, row := range chunk.rows { | |||
| 137 | + | if !yield(row) { | |||
| 138 | + | return | |||
| 139 | + | } | |||
| 140 | + | } | |||
| 141 | + | } | |||
| 142 | + | } | |||
| 143 | + | ||||
| 132 | // Row returns a copy of the row at i, or nil when i is out of range. | 144 | // Row returns a copy of the row at i, or nil when i is out of range. | ||
| 133 | func (v HistoryView) Row(i int) []renderer.Cell { | 145 | func (v HistoryView) Row(i int) []renderer.Cell { | ||
| 134 | if i < 0 { | 146 | if i < 0 { | ||