perf(copy): read row widths without decoding rows
Vertical navigation only needs the row width to clamp the preferred column, but decoded the whole row to read its length. Use RowWidth, and decode once in LineText instead of twice.
- commit
- f3a87678c665a4d439934c72f7c3a217838cf4cc
- author
- brice · · 2026-10-08 07:09 UTC
- committer
- brice · · 2026-10-08 08:12 UTC
- parent
- 9ff586d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +51 -5
M internal/usecase/copy/document.go +2 -1
| @@ -241,7 +241,8 @@ func (d *Document) NextWordEnd(pos Pos) (Pos, bool) { | |||
| 241 | 241 | // LineText returns row text without duplicated continuation cells and retains | |
| 242 | 242 | // the established copy-mode behavior of trimming trailing blank spaces. | |
| 243 | 243 | func (d *Document) LineText(row int) string { | |
| 244 | - | return strings.TrimRight(d.cellsText(d.Row(row), 0, len(d.Row(row))-1), " ") | |
| 244 | + | cells := d.Row(row) | |
| 245 | + | return strings.TrimRight(d.cellsText(cells, 0, len(cells)-1), " ") | |
| 245 | 246 | } | |
| 246 | 247 | ||
| 247 | 248 | // Extract renders inclusive ranges as copyable text. Every emitted line drops | |
| @@ -241,7 +241,8 @@ func (d *Document) NextWordEnd(pos Pos) (Pos, bool) { | |||||
| 241 | // LineText returns row text without duplicated continuation cells and retains | 241 | // LineText returns row text without duplicated continuation cells and retains | ||
| 242 | // the established copy-mode behavior of trimming trailing blank spaces. | 242 | // the established copy-mode behavior of trimming trailing blank spaces. | ||
| 243 | func (d *Document) LineText(row int) string { | 243 | func (d *Document) LineText(row int) string { | ||
| 244 | - | return strings.TrimRight(d.cellsText(d.Row(row), 0, len(d.Row(row))-1), " ") | 244 | + | cells := d.Row(row) |
| 245 | + | return strings.TrimRight(d.cellsText(cells, 0, len(cells)-1), " ") | |||
| 245 | } | 246 | } | ||
| 246 | 247 | ||||
| 247 | // Extract renders inclusive ranges as copyable text. Every emitted line drops | 248 | // Extract renders inclusive ranges as copyable text. Every emitted line drops | ||
M internal/usecase/copy/navigator.go +3 -3
| @@ -140,11 +140,11 @@ func (n *Navigator) moveVertical(doc *Document, row int) bool { | |||
| 140 | 140 | } | |
| 141 | 141 | ||
| 142 | 142 | func navigatorPosOnRow(doc *Document, row, col int) (Pos, bool) { | |
| 143 | - | cells := doc.Row(row) | |
| 144 | - | if len(cells) == 0 { | |
| 143 | + | width := doc.RowWidth(row) | |
| 144 | + | if width == 0 { | |
| 145 | 145 | return doc.Normalize(Pos{Row: row, Col: 0}) | |
| 146 | 146 | } | |
| 147 | - | col = min(max(col, 0), len(cells)-1) | |
| 147 | + | col = min(max(col, 0), width-1) | |
| 148 | 148 | return doc.Normalize(Pos{Row: row, Col: col}) | |
| 149 | 149 | } | |
| 150 | 150 | ||
| @@ -140,11 +140,11 @@ func (n *Navigator) moveVertical(doc *Document, row int) bool { | |||||
| 140 | } | 140 | } | ||
| 141 | 141 | ||||
| 142 | func navigatorPosOnRow(doc *Document, row, col int) (Pos, bool) { | 142 | func navigatorPosOnRow(doc *Document, row, col int) (Pos, bool) { | ||
| 143 | - | cells := doc.Row(row) | 143 | + | width := doc.RowWidth(row) |
| 144 | - | if len(cells) == 0 { | 144 | + | if width == 0 { |
| 145 | return doc.Normalize(Pos{Row: row, Col: 0}) | 145 | return doc.Normalize(Pos{Row: row, Col: 0}) | ||
| 146 | } | 146 | } | ||
| 147 | - | col = min(max(col, 0), len(cells)-1) | 147 | + | col = min(max(col, 0), width-1) |
| 148 | return doc.Normalize(Pos{Row: row, Col: col}) | 148 | return doc.Normalize(Pos{Row: row, Col: col}) | ||
| 149 | } | 149 | } | ||
| 150 | 150 | ||||
M internal/usecase/copy/navigator_test.go +46 -1
| @@ -3,7 +3,8 @@ package copy | |||
| 3 | 3 | import ( | |
| 4 | 4 | "testing" | |
| 5 | 5 | ||
| 6 | - | renderer "github.com/bnema/vev-vt" | |
| 6 | + | vt "github.com/bnema/vev-vt" | |
| 7 | + | renderer "github.com/bnema/vev-vt/ansi" | |
| 7 | 8 | "github.com/stretchr/testify/require" | |
| 8 | 9 | ) | |
| 9 | 10 | ||
| @@ -136,3 +137,47 @@ func TestNavigatorEmptyTopBottomPageAndSet(t *testing.T) { | |||
| 136 | 137 | require.Equal(t, Pos{Row: 2, Col: 0}, navigator.Pos) | |
| 137 | 138 | }) | |
| 138 | 139 | } | |
| 140 | + | ||
| 141 | + | func TestNavigatorVerticalMovesDoNotAllocate(t *testing.T) { | |
| 142 | + | history := vt.NewHistory(vt.HistoryConfig{MaxRows: 8, ChunkRows: 4}) | |
| 143 | + | for _, text := range []string{"abcdef", "xy", "界z", "", "omega"} { | |
| 144 | + | cells := documentCells(text) | |
| 145 | + | if text == "界z" { | |
| 146 | + | cells = []renderer.Cell{{Rune: '界'}, {Continuation: true}, {Rune: 'z'}} | |
| 147 | + | } | |
| 148 | + | require.NoError(t, history.Append(cells, vt.LineBound{End: len(cells)})) | |
| 149 | + | } | |
| 150 | + | // Rows 0-4 come from history and rows 5-6 from the live screen. | |
| 151 | + | doc := NewDocument(NewSnapshot(history, renderer.NewFrame(6, 2), nil, nil), " -_@") | |
| 152 | + | require.Equal(t, 7, doc.Len()) | |
| 153 | + | ||
| 154 | + | tests := []struct { | |
| 155 | + | name string | |
| 156 | + | move func(*Navigator, *Document) bool | |
| 157 | + | from Pos | |
| 158 | + | pref int | |
| 159 | + | want Pos | |
| 160 | + | }{ | |
| 161 | + | {"down clamps to short row", (*Navigator).Down, Pos{Row: 0, Col: 5}, 5, Pos{Row: 1, Col: 1}}, | |
| 162 | + | {"down lands on wide head", (*Navigator).Down, Pos{Row: 1, Col: 1}, 1, Pos{Row: 2, Col: 0}}, | |
| 163 | + | {"down onto empty row", (*Navigator).Down, Pos{Row: 2, Col: 2}, 2, Pos{Row: 3, Col: 0}}, | |
| 164 | + | {"up from empty row", (*Navigator).Up, Pos{Row: 3, Col: 0}, 4, Pos{Row: 2, Col: 2}}, | |
| 165 | + | {"down into history tail", (*Navigator).Down, Pos{Row: 3, Col: 0}, 9, Pos{Row: 4, Col: 4}}, | |
| 166 | + | {"down into live screen", (*Navigator).Down, Pos{Row: 4, Col: 4}, 4, Pos{Row: 5, Col: 4}}, | |
| 167 | + | {"up out of live screen", (*Navigator).Up, Pos{Row: 5, Col: 4}, 4, Pos{Row: 4, Col: 4}}, | |
| 168 | + | {"page down clamps to last row", func(n *Navigator, d *Document) bool { return n.Page(d, 100) }, Pos{Row: 0, Col: 2}, 2, Pos{Row: 6, Col: 2}}, | |
| 169 | + | {"page up clamps to first row", func(n *Navigator, d *Document) bool { return n.Page(d, -100) }, Pos{Row: 6, Col: 2}, 2, Pos{Row: 0, Col: 2}}, | |
| 170 | + | } | |
| 171 | + | for _, tt := range tests { | |
| 172 | + | t.Run(tt.name, func(t *testing.T) { | |
| 173 | + | navigator := &Navigator{} | |
| 174 | + | allocs := testing.AllocsPerRun(100, func() { | |
| 175 | + | *navigator = Navigator{Pos: tt.from, PreferredCol: tt.pref} | |
| 176 | + | if !tt.move(navigator, doc) || navigator.Pos != tt.want { | |
| 177 | + | t.Fatalf("move() = %v, want %v", navigator.Pos, tt.want) | |
| 178 | + | } | |
| 179 | + | }) | |
| 180 | + | require.Zero(t, allocs, "vertical navigation must not decode rows") | |
| 181 | + | }) | |
| 182 | + | } | |
| 183 | + | } | |
| @@ -3,7 +3,8 @@ package copy | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "testing" | 4 | "testing" | ||
| 5 | 5 | ||||
| 6 | - | renderer "github.com/bnema/vev-vt" | 6 | + | vt "github.com/bnema/vev-vt" |
| 7 | + | renderer "github.com/bnema/vev-vt/ansi" | |||
| 7 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| 8 | ) | 9 | ) | ||
| 9 | 10 | ||||
| @@ -136,3 +137,47 @@ func TestNavigatorEmptyTopBottomPageAndSet(t *testing.T) { | |||||
| 136 | require.Equal(t, Pos{Row: 2, Col: 0}, navigator.Pos) | 137 | require.Equal(t, Pos{Row: 2, Col: 0}, navigator.Pos) | ||
| 137 | }) | 138 | }) | ||
| 138 | } | 139 | } | ||
| 140 | + | ||||
| 141 | + | func TestNavigatorVerticalMovesDoNotAllocate(t *testing.T) { | |||
| 142 | + | history := vt.NewHistory(vt.HistoryConfig{MaxRows: 8, ChunkRows: 4}) | |||
| 143 | + | for _, text := range []string{"abcdef", "xy", "界z", "", "omega"} { | |||
| 144 | + | cells := documentCells(text) | |||
| 145 | + | if text == "界z" { | |||
| 146 | + | cells = []renderer.Cell{{Rune: '界'}, {Continuation: true}, {Rune: 'z'}} | |||
| 147 | + | } | |||
| 148 | + | require.NoError(t, history.Append(cells, vt.LineBound{End: len(cells)})) | |||
| 149 | + | } | |||
| 150 | + | // Rows 0-4 come from history and rows 5-6 from the live screen. | |||
| 151 | + | doc := NewDocument(NewSnapshot(history, renderer.NewFrame(6, 2), nil, nil), " -_@") | |||
| 152 | + | require.Equal(t, 7, doc.Len()) | |||
| 153 | + | ||||
| 154 | + | tests := []struct { | |||
| 155 | + | name string | |||
| 156 | + | move func(*Navigator, *Document) bool | |||
| 157 | + | from Pos | |||
| 158 | + | pref int | |||
| 159 | + | want Pos | |||
| 160 | + | }{ | |||
| 161 | + | {"down clamps to short row", (*Navigator).Down, Pos{Row: 0, Col: 5}, 5, Pos{Row: 1, Col: 1}}, | |||
| 162 | + | {"down lands on wide head", (*Navigator).Down, Pos{Row: 1, Col: 1}, 1, Pos{Row: 2, Col: 0}}, | |||
| 163 | + | {"down onto empty row", (*Navigator).Down, Pos{Row: 2, Col: 2}, 2, Pos{Row: 3, Col: 0}}, | |||
| 164 | + | {"up from empty row", (*Navigator).Up, Pos{Row: 3, Col: 0}, 4, Pos{Row: 2, Col: 2}}, | |||
| 165 | + | {"down into history tail", (*Navigator).Down, Pos{Row: 3, Col: 0}, 9, Pos{Row: 4, Col: 4}}, | |||
| 166 | + | {"down into live screen", (*Navigator).Down, Pos{Row: 4, Col: 4}, 4, Pos{Row: 5, Col: 4}}, | |||
| 167 | + | {"up out of live screen", (*Navigator).Up, Pos{Row: 5, Col: 4}, 4, Pos{Row: 4, Col: 4}}, | |||
| 168 | + | {"page down clamps to last row", func(n *Navigator, d *Document) bool { return n.Page(d, 100) }, Pos{Row: 0, Col: 2}, 2, Pos{Row: 6, Col: 2}}, | |||
| 169 | + | {"page up clamps to first row", func(n *Navigator, d *Document) bool { return n.Page(d, -100) }, Pos{Row: 6, Col: 2}, 2, Pos{Row: 0, Col: 2}}, | |||
| 170 | + | } | |||
| 171 | + | for _, tt := range tests { | |||
| 172 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 173 | + | navigator := &Navigator{} | |||
| 174 | + | allocs := testing.AllocsPerRun(100, func() { | |||
| 175 | + | *navigator = Navigator{Pos: tt.from, PreferredCol: tt.pref} | |||
| 176 | + | if !tt.move(navigator, doc) || navigator.Pos != tt.want { | |||
| 177 | + | t.Fatalf("move() = %v, want %v", navigator.Pos, tt.want) | |||
| 178 | + | } | |||
| 179 | + | }) | |||
| 180 | + | require.Zero(t, allocs, "vertical navigation must not decode rows") | |||
| 181 | + | }) | |||
| 182 | + | } | |||
| 183 | + | } | |||