fix(history): clear evicted tail cells, bound scratch maps, harden tests
- evictOldest clears the evicted row's cells before reslicing tailCells so recycled backing storage no longer pins their payload strings. - newHistoryChunksScratch takes a *map and replaces/drops it whenever any chunk (not only the last) grew it past maxRetainedStyleScratch; sealTail also releases oversized tailPageStyles/tailPagePayloads immediately. - rowIDSet.begin resets the collect scratch at the start of each view so an early invalid return cannot leak IDs into a later commit. - Accounting test is no longer circular: non-divisible chunks, row/byte eviction, SetLimits shrink, payload rows, width changes and >1024-style pages, checked after every append against chunk metadata and an independent rebuild of every live chunk. (A fresh history re-fed the same rows can legitimately differ since pages depend on seal boundaries; this is unchanged from the base commit.) Added eviction-prefix, scratch-drop and rowIDSet.begin tests.
- commit
- 9dd242499144ff399fc1e803bfa8f0e4f9d9c707
- author
- brice · · 2026-10-02 09:46 UTC
- parent
- efce2f7
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +183 -42
M history.go +29 -12
| @@ -71,8 +71,20 @@ func newHistoryChunks(rows [][]renderer.Cell, bounds []LineBound, rowIDs []RowID | |||
| 71 | 71 | const maxRetainedStyleScratch = 1024 | |
| 72 | 72 | ||
| 73 | 73 | // newHistoryChunksScratch is newHistoryChunks with an optional caller-owned | |
| 74 | - | // map reused (cleared) across chunks instead of allocating one per chunk. | |
| 75 | - | func newHistoryChunksScratch(rows [][]renderer.Cell, bounds []LineBound, rowIDs []RowID, lastStyleRow map[renderer.Style]int) []*HistoryChunk { | |
| 74 | + | // map (scratch may be nil) reused, cleared, across chunks instead of allocating | |
| 75 | + | // one per chunk. The map is replaced before any reuse if the previous chunk | |
| 76 | + | // grew it past maxRetainedStyleScratch, and dropped on return if the last one | |
| 77 | + | // did, so one high-cardinality chunk cannot pin a large map. | |
| 78 | + | func newHistoryChunksScratch(rows [][]renderer.Cell, bounds []LineBound, rowIDs []RowID, scratch *map[renderer.Style]int) []*HistoryChunk { | |
| 79 | + | var local map[renderer.Style]int | |
| 80 | + | if scratch == nil { | |
| 81 | + | scratch = &local | |
| 82 | + | } | |
| 83 | + | defer func() { | |
| 84 | + | if len(*scratch) > maxRetainedStyleScratch { | |
| 85 | + | *scratch = nil | |
| 86 | + | } | |
| 87 | + | }() | |
| 76 | 88 | bounds = growBounds(bounds, len(rows)) | |
| 77 | 89 | rowIDs = growRowIDs(rowIDs, len(rows)) | |
| 78 | 90 | var chunks []*HistoryChunk | |
| @@ -84,11 +96,12 @@ func newHistoryChunksScratch(rows [][]renderer.Cell, bounds []LineBound, rowIDs | |||
| 84 | 96 | end++ | |
| 85 | 97 | } | |
| 86 | 98 | frame := renderer.NewFrame(width, end-start) | |
| 87 | - | if lastStyleRow == nil { | |
| 88 | - | lastStyleRow = make(map[renderer.Style]int) | |
| 99 | + | if *scratch == nil || len(*scratch) > maxRetainedStyleScratch { | |
| 100 | + | *scratch = make(map[renderer.Style]int) | |
| 89 | 101 | } else { | |
| 90 | - | clear(lastStyleRow) | |
| 102 | + | clear(*scratch) | |
| 91 | 103 | } | |
| 104 | + | lastStyleRow := *scratch | |
| 92 | 105 | defaultStyle := renderer.DefaultStyle() | |
| 93 | 106 | for i := start; i < end; i++ { | |
| 94 | 107 | frame.WriteRow(i-start, 0, rows[i]) | |
| @@ -555,6 +568,13 @@ func (h *History) sealTail() { | |||
| 555 | 568 | h.tailBytes = 0 | |
| 556 | 569 | h.tailPageWidth = 0 | |
| 557 | 570 | h.tailPageRows = 0 | |
| 571 | + | // Sets are cleared lazily by the next page; release oversized ones now. | |
| 572 | + | if len(h.tailPageStyles) > maxRetainedStyleScratch { | |
| 573 | + | h.tailPageStyles = nil | |
| 574 | + | } | |
| 575 | + | if len(h.tailPagePayloads) > maxRetainedStyleScratch { | |
| 576 | + | h.tailPagePayloads = nil | |
| 577 | + | } | |
| 558 | 578 | } | |
| 559 | 579 | ||
| 560 | 580 | // evictFor discards oldest rows until row can fit both retention budgets. | |
| @@ -601,6 +621,9 @@ func (h *History) evictOldest() { | |||
| 601 | 621 | h.cells -= len(row) | |
| 602 | 622 | h.tail[0] = nil | |
| 603 | 623 | h.tail = h.tail[1:] | |
| 624 | + | // The evicted prefix stays in the recycled backing array; clear it so its | |
| 625 | + | // payload strings are not pinned until the next seal. | |
| 626 | + | clear(row) | |
| 604 | 627 | h.tailCells = h.tailCells[len(row):] | |
| 605 | 628 | h.tailBounds = growBounds(h.tailBounds, len(h.tail)+1)[1:] | |
| 606 | 629 | h.tailIDs = growRowIDs(h.tailIDs, len(h.tail)+1)[1:] | |
| @@ -618,13 +641,7 @@ func (h *History) SealAndView() HistoryView { | |||
| 618 | 641 | ||
| 619 | 642 | func (h *History) tailViewChunks() []*HistoryChunk { | |
| 620 | 643 | if h.cachedTail == nil && len(h.tail) > 0 { | |
| 621 | - | if len(h.styleRowScratch) > maxRetainedStyleScratch { | |
| 622 | - | h.styleRowScratch = nil | |
| 623 | - | } | |
| 624 | - | if h.styleRowScratch == nil { | |
| 625 | - | h.styleRowScratch = make(map[renderer.Style]int) | |
| 626 | - | } | |
| 627 | - | h.cachedTail = newHistoryChunksScratch(h.tail, h.tailBounds, h.tailIDs, h.styleRowScratch) | |
| 644 | + | h.cachedTail = newHistoryChunksScratch(h.tail, h.tailBounds, h.tailIDs, &h.styleRowScratch) | |
| 628 | 645 | } | |
| 629 | 646 | return h.cachedTail | |
| 630 | 647 | } | |
| @@ -71,8 +71,20 @@ func newHistoryChunks(rows [][]renderer.Cell, bounds []LineBound, rowIDs []RowID | |||||
| 71 | const maxRetainedStyleScratch = 1024 | 71 | const maxRetainedStyleScratch = 1024 | ||
| 72 | 72 | ||||
| 73 | // newHistoryChunksScratch is newHistoryChunks with an optional caller-owned | 73 | // newHistoryChunksScratch is newHistoryChunks with an optional caller-owned | ||
| 74 | - | // map reused (cleared) across chunks instead of allocating one per chunk. | 74 | + | // map (scratch may be nil) reused, cleared, across chunks instead of allocating |
| 75 | - | func newHistoryChunksScratch(rows [][]renderer.Cell, bounds []LineBound, rowIDs []RowID, lastStyleRow map[renderer.Style]int) []*HistoryChunk { | 75 | + | // one per chunk. The map is replaced before any reuse if the previous chunk |
| 76 | + | // grew it past maxRetainedStyleScratch, and dropped on return if the last one | |||
| 77 | + | // did, so one high-cardinality chunk cannot pin a large map. | |||
| 78 | + | func newHistoryChunksScratch(rows [][]renderer.Cell, bounds []LineBound, rowIDs []RowID, scratch *map[renderer.Style]int) []*HistoryChunk { | |||
| 79 | + | var local map[renderer.Style]int | |||
| 80 | + | if scratch == nil { | |||
| 81 | + | scratch = &local | |||
| 82 | + | } | |||
| 83 | + | defer func() { | |||
| 84 | + | if len(*scratch) > maxRetainedStyleScratch { | |||
| 85 | + | *scratch = nil | |||
| 86 | + | } | |||
| 87 | + | }() | |||
| 76 | bounds = growBounds(bounds, len(rows)) | 88 | bounds = growBounds(bounds, len(rows)) | ||
| 77 | rowIDs = growRowIDs(rowIDs, len(rows)) | 89 | rowIDs = growRowIDs(rowIDs, len(rows)) | ||
| 78 | var chunks []*HistoryChunk | 90 | var chunks []*HistoryChunk | ||
| @@ -84,11 +96,12 @@ func newHistoryChunksScratch(rows [][]renderer.Cell, bounds []LineBound, rowIDs | |||||
| 84 | end++ | 96 | end++ | ||
| 85 | } | 97 | } | ||
| 86 | frame := renderer.NewFrame(width, end-start) | 98 | frame := renderer.NewFrame(width, end-start) | ||
| 87 | - | if lastStyleRow == nil { | 99 | + | if *scratch == nil || len(*scratch) > maxRetainedStyleScratch { |
| 88 | - | lastStyleRow = make(map[renderer.Style]int) | 100 | + | *scratch = make(map[renderer.Style]int) |
| 89 | } else { | 101 | } else { | ||
| 90 | - | clear(lastStyleRow) | 102 | + | clear(*scratch) |
| 91 | } | 103 | } | ||
| 104 | + | lastStyleRow := *scratch | |||
| 92 | defaultStyle := renderer.DefaultStyle() | 105 | defaultStyle := renderer.DefaultStyle() | ||
| 93 | for i := start; i < end; i++ { | 106 | for i := start; i < end; i++ { | ||
| 94 | frame.WriteRow(i-start, 0, rows[i]) | 107 | frame.WriteRow(i-start, 0, rows[i]) | ||
| @@ -555,6 +568,13 @@ func (h *History) sealTail() { | |||||
| 555 | h.tailBytes = 0 | 568 | h.tailBytes = 0 | ||
| 556 | h.tailPageWidth = 0 | 569 | h.tailPageWidth = 0 | ||
| 557 | h.tailPageRows = 0 | 570 | h.tailPageRows = 0 | ||
| 571 | + | // Sets are cleared lazily by the next page; release oversized ones now. | |||
| 572 | + | if len(h.tailPageStyles) > maxRetainedStyleScratch { | |||
| 573 | + | h.tailPageStyles = nil | |||
| 574 | + | } | |||
| 575 | + | if len(h.tailPagePayloads) > maxRetainedStyleScratch { | |||
| 576 | + | h.tailPagePayloads = nil | |||
| 577 | + | } | |||
| 558 | } | 578 | } | ||
| 559 | 579 | ||||
| 560 | // evictFor discards oldest rows until row can fit both retention budgets. | 580 | // evictFor discards oldest rows until row can fit both retention budgets. | ||
| @@ -601,6 +621,9 @@ func (h *History) evictOldest() { | |||||
| 601 | h.cells -= len(row) | 621 | h.cells -= len(row) | ||
| 602 | h.tail[0] = nil | 622 | h.tail[0] = nil | ||
| 603 | h.tail = h.tail[1:] | 623 | h.tail = h.tail[1:] | ||
| 624 | + | // The evicted prefix stays in the recycled backing array; clear it so its | |||
| 625 | + | // payload strings are not pinned until the next seal. | |||
| 626 | + | clear(row) | |||
| 604 | h.tailCells = h.tailCells[len(row):] | 627 | h.tailCells = h.tailCells[len(row):] | ||
| 605 | h.tailBounds = growBounds(h.tailBounds, len(h.tail)+1)[1:] | 628 | h.tailBounds = growBounds(h.tailBounds, len(h.tail)+1)[1:] | ||
| 606 | h.tailIDs = growRowIDs(h.tailIDs, len(h.tail)+1)[1:] | 629 | h.tailIDs = growRowIDs(h.tailIDs, len(h.tail)+1)[1:] | ||
| @@ -618,13 +641,7 @@ func (h *History) SealAndView() HistoryView { | |||||
| 618 | 641 | ||||
| 619 | func (h *History) tailViewChunks() []*HistoryChunk { | 642 | func (h *History) tailViewChunks() []*HistoryChunk { | ||
| 620 | if h.cachedTail == nil && len(h.tail) > 0 { | 643 | if h.cachedTail == nil && len(h.tail) > 0 { | ||
| 621 | - | if len(h.styleRowScratch) > maxRetainedStyleScratch { | 644 | + | h.cachedTail = newHistoryChunksScratch(h.tail, h.tailBounds, h.tailIDs, &h.styleRowScratch) |
| 622 | - | h.styleRowScratch = nil | |||
| 623 | - | } | |||
| 624 | - | if h.styleRowScratch == nil { | |||
| 625 | - | h.styleRowScratch = make(map[renderer.Style]int) | |||
| 626 | - | } | |||
| 627 | - | h.cachedTail = newHistoryChunksScratch(h.tail, h.tailBounds, h.tailIDs, h.styleRowScratch) | |||
| 628 | } | 645 | } | ||
| 629 | return h.cachedTail | 646 | return h.cachedTail | ||
| 630 | } | 647 | } | ||
M history_codec.go +1 -1
| @@ -280,7 +280,7 @@ func parseHistory(data []byte, populate bool) (HistoryView, historyDecodeStats, | |||
| 280 | 280 | return invalid() | |
| 281 | 281 | } | |
| 282 | 282 | usedPayloads := make([]bool, npayloads) | |
| 283 | - | seenIDs.reserve(int(rows)) | |
| 283 | + | seenIDs.begin(int(rows)) | |
| 284 | 284 | rowIDs := make([]RowID, rows) | |
| 285 | 285 | bounds := make([]LineBound, rows) | |
| 286 | 286 | for row := range rows { | |
| @@ -280,7 +280,7 @@ func parseHistory(data []byte, populate bool) (HistoryView, historyDecodeStats, | |||||
| 280 | return invalid() | 280 | return invalid() | ||
| 281 | } | 281 | } | ||
| 282 | usedPayloads := make([]bool, npayloads) | 282 | usedPayloads := make([]bool, npayloads) | ||
| 283 | - | seenIDs.reserve(int(rows)) | 283 | + | seenIDs.begin(int(rows)) |
| 284 | rowIDs := make([]RowID, rows) | 284 | rowIDs := make([]RowID, rows) | ||
| 285 | bounds := make([]LineBound, rows) | 285 | bounds := make([]LineBound, rows) | ||
| 286 | for row := range rows { | 286 | for row := range rows { | ||
M history_rowids.go +3 -0
| @@ -17,6 +17,9 @@ type rowIDSet struct { | |||
| 17 | 17 | scratch []RowID | |
| 18 | 18 | } | |
| 19 | 19 | ||
| 20 | + | // begin discards any IDs collected but not committed and reserves room for n. | |
| 21 | + | func (s *rowIDSet) begin(n int) { s.scratch = slices.Grow(s.scratch[:0], n) } | |
| 22 | + | ||
| 20 | 23 | // reserve makes room for n more collected IDs. | |
| 21 | 24 | func (s *rowIDSet) reserve(n int) { s.scratch = slices.Grow(s.scratch, n) } | |
| 22 | 25 | ||
| @@ -17,6 +17,9 @@ type rowIDSet struct { | |||||
| 17 | scratch []RowID | 17 | scratch []RowID | ||
| 18 | } | 18 | } | ||
| 19 | 19 | ||||
| 20 | + | // begin discards any IDs collected but not committed and reserves room for n. | |||
| 21 | + | func (s *rowIDSet) begin(n int) { s.scratch = slices.Grow(s.scratch[:0], n) } | |||
| 22 | + | ||||
| 20 | // reserve makes room for n more collected IDs. | 23 | // reserve makes room for n more collected IDs. | ||
| 21 | func (s *rowIDSet) reserve(n int) { s.scratch = slices.Grow(s.scratch, n) } | 24 | func (s *rowIDSet) reserve(n int) { s.scratch = slices.Grow(s.scratch, n) } | ||
| 22 | 25 | ||||
M history_tail_reuse_test.go +149 -29
| @@ -74,11 +74,11 @@ func TestHistoryTailStorageIsRecycledAcrossSeals(t *testing.T) { | |||
| 74 | 74 | require.Empty(t, h.tail) | |
| 75 | 75 | require.Zero(t, len(h.tailCells)) | |
| 76 | 76 | require.NotZero(t, cap(h.tailCells), "seal must keep tail storage for the next chunk") | |
| 77 | - | first := h.tailCells[:1][0:1] | |
| 77 | + | first := &h.tailCells[:1][0] | |
| 78 | 78 | for i := 4; i < 8; i++ { | |
| 79 | 79 | require.NoError(t, h.Append(historyRow(fmt.Sprintf("r%03d", i)), LineBound{End: 4})) | |
| 80 | 80 | } | |
| 81 | - | require.Same(t, &first[0], &h.tailCells[:1][0], "storage reallocated across seals") | |
| 81 | + | require.Same(t, first, &h.tailCells[:1][0], "storage reallocated across seals") | |
| 82 | 82 | // A reused backing array must not pin payload strings of sealed rows. | |
| 83 | 83 | payload, err := renderer.NewCellPayload("é\u0301", "https://example.test") | |
| 84 | 84 | require.NoError(t, err) | |
| @@ -93,40 +93,125 @@ func TestHistoryTailStorageIsRecycledAcrossSeals(t *testing.T) { | |||
| 93 | 93 | require.NoError(t, h.chunks[len(h.chunks)-1].CheckInvariants()) | |
| 94 | 94 | } | |
| 95 | 95 | ||
| 96 | - | // Accounting after style/payload set reuse must match a history that has never | |
| 97 | - | // sealed anything. | |
| 98 | - | func TestHistoryLogicalBytesIndependentOfSetReuse(t *testing.T) { | |
| 99 | - | payload, err := renderer.NewCellPayload("x", "https://example.test") | |
| 96 | + | // Eviction from a partial tail must not leave the evicted rows' payload | |
| 97 | + | // strings pinned in the recycled backing array's prefix. | |
| 98 | + | func TestHistoryTailEvictionClearsEvictedPrefix(t *testing.T) { | |
| 99 | + | payload, err := renderer.NewCellPayload("é\u0301", "https://example.test") | |
| 100 | 100 | require.NoError(t, err) | |
| 101 | - | rows := make([][]renderer.Cell, 40) | |
| 102 | - | for i := range rows { | |
| 103 | - | rows[i] = styledRow("abcd", i%7+1) | |
| 104 | - | if i%3 == 0 { | |
| 105 | - | rows[i][1].Payload = payload | |
| 106 | - | } | |
| 101 | + | rowFor := func(i int) []renderer.Cell { | |
| 102 | + | row := historyRow("abcd") | |
| 103 | + | row[i%4].Payload = payload | |
| 104 | + | return row | |
| 107 | 105 | } | |
| 108 | - | reused := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1 << 20, ChunkRows: 5}) | |
| 109 | - | whole := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1 << 20, ChunkRows: 100}) | |
| 110 | - | for _, row := range rows { | |
| 111 | - | require.NoError(t, reused.Append(row, LineBound{End: 4})) | |
| 112 | - | require.NoError(t, whole.Append(row, LineBound{End: 4})) | |
| 106 | + | // The byte budget evicts from the tail (ChunkRows is large, so it never | |
| 107 | + | // seals); the 400-cell preallocation leaves room for many evictions in | |
| 108 | + | // one backing array before it would be reallocated. | |
| 109 | + | h := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1500, ChunkRows: 100}) | |
| 110 | + | require.NoError(t, h.Append(rowFor(0), LineBound{End: 4})) | |
| 111 | + | orig := h.tailCells[:cap(h.tailCells)] // full original backing array | |
| 112 | + | evicted := 0 | |
| 113 | + | for i := 1; i < 40 && len(h.chunks) == 0; i++ { | |
| 114 | + | before := h.Len() | |
| 115 | + | require.NoError(t, h.Append(rowFor(i), LineBound{End: 4})) | |
| 116 | + | if cap(h.tailCells) > len(orig) || len(h.tailCells) == 0 { | |
| 117 | + | break | |
| 118 | + | } | |
| 119 | + | liveStart := len(orig) - cap(h.tailCells) | |
| 120 | + | if &orig[liveStart] != &h.tailCells[:1][0] { | |
| 121 | + | break // storage was reallocated; the old array is no longer ours | |
| 122 | + | } | |
| 123 | + | if h.Len() <= before { | |
| 124 | + | evicted++ | |
| 125 | + | } | |
| 126 | + | liveEnd := liveStart + len(h.tailCells) | |
| 127 | + | for j, c := range orig { | |
| 128 | + | if j < liveStart || j >= liveEnd { | |
| 129 | + | require.True(t, c.Payload.Empty(), "cell %d outside live window [%d,%d) pins payload", j, liveStart, liveEnd) | |
| 130 | + | } | |
| 131 | + | } | |
| 113 | 132 | } | |
| 114 | - | // Re-encode each retained chunk from scratch and compare accounting. | |
| 115 | - | var fresh uint64 | |
| 116 | - | for _, c := range reused.View().chunks { | |
| 117 | - | rebuilt := newHistoryChunks(c.rowsForTest(), c.bounds, c.rowIDs) | |
| 118 | - | fresh += historyChunksLogicalBytes(rebuilt) | |
| 133 | + | require.GreaterOrEqual(t, evicted, 2, "test did not exercise tail eviction") | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | // checkHistoryAccounting compares the incrementally maintained LogicalBytes | |
| 137 | + | // with (1) the sum of the live chunks' own metadata and (2) an independent | |
| 138 | + | // recomputation: every live chunk is rebuilt from its decoded rows with a | |
| 139 | + | // fresh newHistoryChunks call (no recycled scratch state). Logical bytes depend | |
| 140 | + | // on the page layout, so the reference keeps each chunk's boundaries. | |
| 141 | + | func checkHistoryAccounting(t *testing.T, h *History) { | |
| 142 | + | t.Helper() | |
| 143 | + | view := h.View() | |
| 144 | + | require.Equal(t, historyChunksLogicalBytes(view.chunks), h.LogicalBytes(), "rows=%d", h.Len()) | |
| 145 | + | require.Equal(t, h.Len(), view.Len()) | |
| 146 | + | var rebuilt uint64 | |
| 147 | + | for _, c := range view.chunks { | |
| 148 | + | rows := make([][]renderer.Cell, c.len()) | |
| 149 | + | for i := range rows { | |
| 150 | + | rows[i] = c.row(i) | |
| 151 | + | } | |
| 152 | + | rebuilt += historyChunksLogicalBytes(newHistoryChunks(rows, c.bounds, c.rowIDs)) | |
| 119 | 153 | } | |
| 120 | - | require.Equal(t, fresh, reused.LogicalBytes()) | |
| 121 | - | require.Equal(t, rangeTexts(t, whole.View()), rangeTexts(t, reused.View())) | |
| 154 | + | require.Equal(t, rebuilt, h.LogicalBytes(), "independent rebuild rows=%d", h.Len()) | |
| 122 | 155 | } | |
| 123 | 156 | ||
| 124 | - | func (c *HistoryChunk) rowsForTest() [][]renderer.Cell { | |
| 125 | - | rows := make([][]renderer.Cell, c.len()) | |
| 126 | - | for i := range rows { | |
| 127 | - | rows[i] = c.row(i) | |
| 157 | + | // Accounting must stay exact after seals recycle the style and payload sets, | |
| 158 | + | // through eviction, SetLimits shrinking, payload rows, width changes and pages | |
| 159 | + | // with more than maxRetainedStyleScratch distinct styles. | |
| 160 | + | func TestHistoryLogicalBytesIndependentOfSetReuse(t *testing.T) { | |
| 161 | + | payload, err := renderer.NewCellPayload("x", "https://example.test") | |
| 162 | + | require.NoError(t, err) | |
| 163 | + | payload2, err := renderer.NewCellPayload("y", "") | |
| 164 | + | require.NoError(t, err) | |
| 165 | + | manyStyles := func(i int) []renderer.Cell { // 1100 distinct styles in one row | |
| 166 | + | row := make([]renderer.Cell, 1100) | |
| 167 | + | for x := range row { | |
| 168 | + | row[x] = renderer.Cell{Rune: 'z', Style: renderer.Style{HasForegroundRGB: true, ForegroundRGB: renderer.RGB{R: uint8(x), G: uint8(x >> 8), B: uint8(i)}}} | |
| 169 | + | } | |
| 170 | + | return row | |
| 171 | + | } | |
| 172 | + | rowFor := func(i int) []renderer.Cell { | |
| 173 | + | switch { | |
| 174 | + | case i%11 == 5: | |
| 175 | + | return manyStyles(i) | |
| 176 | + | case i%4 == 0: | |
| 177 | + | row := styledRow("abcd", i%7+1) | |
| 178 | + | row[1].Payload = payload | |
| 179 | + | row[2].Payload = payload2 | |
| 180 | + | return row | |
| 181 | + | case i%3 == 0: | |
| 182 | + | return styledRow("abcdef", i%5+1) // width change forces a new page | |
| 183 | + | } | |
| 184 | + | return styledRow("abcd", i%7+1) | |
| 185 | + | } | |
| 186 | + | for _, tc := range []struct { | |
| 187 | + | name string | |
| 188 | + | cfg HistoryConfig | |
| 189 | + | }{ | |
| 190 | + | {"non-divisible chunks", HistoryConfig{MaxRows: 200, MaxBytes: 1 << 30, ChunkRows: 7}}, | |
| 191 | + | {"row eviction", HistoryConfig{MaxRows: 9, MaxBytes: 1 << 30, ChunkRows: 4}}, | |
| 192 | + | {"byte eviction", HistoryConfig{MaxRows: 200, MaxBytes: 30 << 10, ChunkRows: 5}}, | |
| 193 | + | {"tiny bytes", HistoryConfig{MaxRows: 200, MaxBytes: 1500, ChunkRows: 3}}, | |
| 194 | + | } { | |
| 195 | + | t.Run(tc.name, func(t *testing.T) { | |
| 196 | + | h := NewHistory(tc.cfg) | |
| 197 | + | appendAt := func(i int) { | |
| 198 | + | row := rowFor(i) | |
| 199 | + | if err := h.Append(row, LineBound{End: len(row)}); err != nil { | |
| 200 | + | require.ErrorIs(t, err, ErrHistoryRowTooLarge) | |
| 201 | + | } | |
| 202 | + | checkHistoryAccounting(t, h) | |
| 203 | + | } | |
| 204 | + | for i := range 60 { | |
| 205 | + | appendAt(i) | |
| 206 | + | } | |
| 207 | + | shrunk := HistoryConfig{MaxRows: 5, MaxBytes: tc.cfg.MaxBytes, ChunkRows: 2} | |
| 208 | + | require.NoError(t, h.SetLimits(shrunk)) | |
| 209 | + | checkHistoryAccounting(t, h) | |
| 210 | + | for i := 60; i < 75; i++ { | |
| 211 | + | appendAt(i) | |
| 212 | + | } | |
| 213 | + | }) | |
| 128 | 214 | } | |
| 129 | - | return rows | |
| 130 | 215 | } | |
| 131 | 216 | ||
| 132 | 217 | // rowIDSet must accept and reject exactly what a plain map would, including | |
| @@ -192,3 +277,38 @@ func TestRestoreAcceptsOutOfOrderDistinctRowIDsAcrossBlobs(t *testing.T) { | |||
| 192 | 277 | _, err = HistoryFromBlobs(HistoryConfig{MaxRows: 16, ChunkRows: 2}, [][]byte{sealed[1], sealed[0], sealed[1]}, tail) | |
| 193 | 278 | require.Error(t, err) | |
| 194 | 279 | } | |
| 280 | + | ||
| 281 | + | // A chunk with more than maxRetainedStyleScratch styles must not leave its | |
| 282 | + | // large scratch map pinned, whether it is the last or an earlier chunk. | |
| 283 | + | func TestHistoryStyleScratchDroppedAfterHighCardinalityChunk(t *testing.T) { | |
| 284 | + | row := make([]renderer.Cell, 1100) | |
| 285 | + | for x := range row { | |
| 286 | + | row[x] = renderer.Cell{Rune: 'z', Style: renderer.Style{HasForegroundRGB: true, ForegroundRGB: renderer.RGB{R: uint8(x), G: uint8(x >> 8), B: 1}}} | |
| 287 | + | } | |
| 288 | + | h := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1 << 30, ChunkRows: 4}) | |
| 289 | + | for range 4 { // seals one high-cardinality chunk | |
| 290 | + | require.NoError(t, h.Append(row, LineBound{End: len(row)})) | |
| 291 | + | } | |
| 292 | + | require.LessOrEqual(t, len(h.styleRowScratch), maxRetainedStyleScratch) | |
| 293 | + | require.LessOrEqual(t, len(h.tailPageStyles), maxRetainedStyleScratch+1) | |
| 294 | + | for range 3 { | |
| 295 | + | require.NoError(t, h.Append(historyRow("abcd"), LineBound{End: 4})) | |
| 296 | + | } | |
| 297 | + | _ = h.View() | |
| 298 | + | require.LessOrEqual(t, len(h.styleRowScratch), maxRetainedStyleScratch) | |
| 299 | + | } | |
| 300 | + | ||
| 301 | + | // Early false returns from validation must not leak collected IDs into a later | |
| 302 | + | // commit. | |
| 303 | + | func TestRowIDSetBeginDiscardsUncommittedIDs(t *testing.T) { | |
| 304 | + | var set rowIDSet | |
| 305 | + | set.begin(0) | |
| 306 | + | set.collect(1) | |
| 307 | + | set.collect(2) // abandoned without commit, as on an early invalid return | |
| 308 | + | set.begin(0) | |
| 309 | + | set.collect(2) | |
| 310 | + | require.True(t, set.commit()) | |
| 311 | + | set.begin(0) | |
| 312 | + | set.collect(1) | |
| 313 | + | require.True(t, set.commit()) | |
| 314 | + | } | |
| @@ -74,11 +74,11 @@ func TestHistoryTailStorageIsRecycledAcrossSeals(t *testing.T) { | |||||
| 74 | require.Empty(t, h.tail) | 74 | require.Empty(t, h.tail) | ||
| 75 | require.Zero(t, len(h.tailCells)) | 75 | require.Zero(t, len(h.tailCells)) | ||
| 76 | require.NotZero(t, cap(h.tailCells), "seal must keep tail storage for the next chunk") | 76 | require.NotZero(t, cap(h.tailCells), "seal must keep tail storage for the next chunk") | ||
| 77 | - | first := h.tailCells[:1][0:1] | 77 | + | first := &h.tailCells[:1][0] |
| 78 | for i := 4; i < 8; i++ { | 78 | for i := 4; i < 8; i++ { | ||
| 79 | require.NoError(t, h.Append(historyRow(fmt.Sprintf("r%03d", i)), LineBound{End: 4})) | 79 | require.NoError(t, h.Append(historyRow(fmt.Sprintf("r%03d", i)), LineBound{End: 4})) | ||
| 80 | } | 80 | } | ||
| 81 | - | require.Same(t, &first[0], &h.tailCells[:1][0], "storage reallocated across seals") | 81 | + | require.Same(t, first, &h.tailCells[:1][0], "storage reallocated across seals") |
| 82 | // A reused backing array must not pin payload strings of sealed rows. | 82 | // A reused backing array must not pin payload strings of sealed rows. | ||
| 83 | payload, err := renderer.NewCellPayload("é\u0301", "https://example.test") | 83 | payload, err := renderer.NewCellPayload("é\u0301", "https://example.test") | ||
| 84 | require.NoError(t, err) | 84 | require.NoError(t, err) | ||
| @@ -93,40 +93,125 @@ func TestHistoryTailStorageIsRecycledAcrossSeals(t *testing.T) { | |||||
| 93 | require.NoError(t, h.chunks[len(h.chunks)-1].CheckInvariants()) | 93 | require.NoError(t, h.chunks[len(h.chunks)-1].CheckInvariants()) | ||
| 94 | } | 94 | } | ||
| 95 | 95 | ||||
| 96 | - | // Accounting after style/payload set reuse must match a history that has never | 96 | + | // Eviction from a partial tail must not leave the evicted rows' payload |
| 97 | - | // sealed anything. | 97 | + | // strings pinned in the recycled backing array's prefix. |
| 98 | - | func TestHistoryLogicalBytesIndependentOfSetReuse(t *testing.T) { | 98 | + | func TestHistoryTailEvictionClearsEvictedPrefix(t *testing.T) { |
| 99 | - | payload, err := renderer.NewCellPayload("x", "https://example.test") | 99 | + | payload, err := renderer.NewCellPayload("é\u0301", "https://example.test") |
| 100 | require.NoError(t, err) | 100 | require.NoError(t, err) | ||
| 101 | - | rows := make([][]renderer.Cell, 40) | 101 | + | rowFor := func(i int) []renderer.Cell { |
| 102 | - | for i := range rows { | 102 | + | row := historyRow("abcd") |
| 103 | - | rows[i] = styledRow("abcd", i%7+1) | 103 | + | row[i%4].Payload = payload |
| 104 | - | if i%3 == 0 { | 104 | + | return row |
| 105 | - | rows[i][1].Payload = payload | |||
| 106 | - | } | |||
| 107 | } | 105 | } | ||
| 108 | - | reused := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1 << 20, ChunkRows: 5}) | 106 | + | // The byte budget evicts from the tail (ChunkRows is large, so it never |
| 109 | - | whole := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1 << 20, ChunkRows: 100}) | 107 | + | // seals); the 400-cell preallocation leaves room for many evictions in |
| 110 | - | for _, row := range rows { | 108 | + | // one backing array before it would be reallocated. |
| 111 | - | require.NoError(t, reused.Append(row, LineBound{End: 4})) | 109 | + | h := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1500, ChunkRows: 100}) |
| 112 | - | require.NoError(t, whole.Append(row, LineBound{End: 4})) | 110 | + | require.NoError(t, h.Append(rowFor(0), LineBound{End: 4})) |
| 111 | + | orig := h.tailCells[:cap(h.tailCells)] // full original backing array | |||
| 112 | + | evicted := 0 | |||
| 113 | + | for i := 1; i < 40 && len(h.chunks) == 0; i++ { | |||
| 114 | + | before := h.Len() | |||
| 115 | + | require.NoError(t, h.Append(rowFor(i), LineBound{End: 4})) | |||
| 116 | + | if cap(h.tailCells) > len(orig) || len(h.tailCells) == 0 { | |||
| 117 | + | break | |||
| 118 | + | } | |||
| 119 | + | liveStart := len(orig) - cap(h.tailCells) | |||
| 120 | + | if &orig[liveStart] != &h.tailCells[:1][0] { | |||
| 121 | + | break // storage was reallocated; the old array is no longer ours | |||
| 122 | + | } | |||
| 123 | + | if h.Len() <= before { | |||
| 124 | + | evicted++ | |||
| 125 | + | } | |||
| 126 | + | liveEnd := liveStart + len(h.tailCells) | |||
| 127 | + | for j, c := range orig { | |||
| 128 | + | if j < liveStart || j >= liveEnd { | |||
| 129 | + | require.True(t, c.Payload.Empty(), "cell %d outside live window [%d,%d) pins payload", j, liveStart, liveEnd) | |||
| 130 | + | } | |||
| 131 | + | } | |||
| 113 | } | 132 | } | ||
| 114 | - | // Re-encode each retained chunk from scratch and compare accounting. | 133 | + | require.GreaterOrEqual(t, evicted, 2, "test did not exercise tail eviction") |
| 115 | - | var fresh uint64 | 134 | + | } |
| 116 | - | for _, c := range reused.View().chunks { | 135 | + | |
| 117 | - | rebuilt := newHistoryChunks(c.rowsForTest(), c.bounds, c.rowIDs) | 136 | + | // checkHistoryAccounting compares the incrementally maintained LogicalBytes |
| 118 | - | fresh += historyChunksLogicalBytes(rebuilt) | 137 | + | // with (1) the sum of the live chunks' own metadata and (2) an independent |
| 138 | + | // recomputation: every live chunk is rebuilt from its decoded rows with a | |||
| 139 | + | // fresh newHistoryChunks call (no recycled scratch state). Logical bytes depend | |||
| 140 | + | // on the page layout, so the reference keeps each chunk's boundaries. | |||
| 141 | + | func checkHistoryAccounting(t *testing.T, h *History) { | |||
| 142 | + | t.Helper() | |||
| 143 | + | view := h.View() | |||
| 144 | + | require.Equal(t, historyChunksLogicalBytes(view.chunks), h.LogicalBytes(), "rows=%d", h.Len()) | |||
| 145 | + | require.Equal(t, h.Len(), view.Len()) | |||
| 146 | + | var rebuilt uint64 | |||
| 147 | + | for _, c := range view.chunks { | |||
| 148 | + | rows := make([][]renderer.Cell, c.len()) | |||
| 149 | + | for i := range rows { | |||
| 150 | + | rows[i] = c.row(i) | |||
| 151 | + | } | |||
| 152 | + | rebuilt += historyChunksLogicalBytes(newHistoryChunks(rows, c.bounds, c.rowIDs)) | |||
| 119 | } | 153 | } | ||
| 120 | - | require.Equal(t, fresh, reused.LogicalBytes()) | 154 | + | require.Equal(t, rebuilt, h.LogicalBytes(), "independent rebuild rows=%d", h.Len()) |
| 121 | - | require.Equal(t, rangeTexts(t, whole.View()), rangeTexts(t, reused.View())) | |||
| 122 | } | 155 | } | ||
| 123 | 156 | ||||
| 124 | - | func (c *HistoryChunk) rowsForTest() [][]renderer.Cell { | 157 | + | // Accounting must stay exact after seals recycle the style and payload sets, |
| 125 | - | rows := make([][]renderer.Cell, c.len()) | 158 | + | // through eviction, SetLimits shrinking, payload rows, width changes and pages |
| 126 | - | for i := range rows { | 159 | + | // with more than maxRetainedStyleScratch distinct styles. |
| 127 | - | rows[i] = c.row(i) | 160 | + | func TestHistoryLogicalBytesIndependentOfSetReuse(t *testing.T) { |
| 161 | + | payload, err := renderer.NewCellPayload("x", "https://example.test") | |||
| 162 | + | require.NoError(t, err) | |||
| 163 | + | payload2, err := renderer.NewCellPayload("y", "") | |||
| 164 | + | require.NoError(t, err) | |||
| 165 | + | manyStyles := func(i int) []renderer.Cell { // 1100 distinct styles in one row | |||
| 166 | + | row := make([]renderer.Cell, 1100) | |||
| 167 | + | for x := range row { | |||
| 168 | + | row[x] = renderer.Cell{Rune: 'z', Style: renderer.Style{HasForegroundRGB: true, ForegroundRGB: renderer.RGB{R: uint8(x), G: uint8(x >> 8), B: uint8(i)}}} | |||
| 169 | + | } | |||
| 170 | + | return row | |||
| 171 | + | } | |||
| 172 | + | rowFor := func(i int) []renderer.Cell { | |||
| 173 | + | switch { | |||
| 174 | + | case i%11 == 5: | |||
| 175 | + | return manyStyles(i) | |||
| 176 | + | case i%4 == 0: | |||
| 177 | + | row := styledRow("abcd", i%7+1) | |||
| 178 | + | row[1].Payload = payload | |||
| 179 | + | row[2].Payload = payload2 | |||
| 180 | + | return row | |||
| 181 | + | case i%3 == 0: | |||
| 182 | + | return styledRow("abcdef", i%5+1) // width change forces a new page | |||
| 183 | + | } | |||
| 184 | + | return styledRow("abcd", i%7+1) | |||
| 185 | + | } | |||
| 186 | + | for _, tc := range []struct { | |||
| 187 | + | name string | |||
| 188 | + | cfg HistoryConfig | |||
| 189 | + | }{ | |||
| 190 | + | {"non-divisible chunks", HistoryConfig{MaxRows: 200, MaxBytes: 1 << 30, ChunkRows: 7}}, | |||
| 191 | + | {"row eviction", HistoryConfig{MaxRows: 9, MaxBytes: 1 << 30, ChunkRows: 4}}, | |||
| 192 | + | {"byte eviction", HistoryConfig{MaxRows: 200, MaxBytes: 30 << 10, ChunkRows: 5}}, | |||
| 193 | + | {"tiny bytes", HistoryConfig{MaxRows: 200, MaxBytes: 1500, ChunkRows: 3}}, | |||
| 194 | + | } { | |||
| 195 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 196 | + | h := NewHistory(tc.cfg) | |||
| 197 | + | appendAt := func(i int) { | |||
| 198 | + | row := rowFor(i) | |||
| 199 | + | if err := h.Append(row, LineBound{End: len(row)}); err != nil { | |||
| 200 | + | require.ErrorIs(t, err, ErrHistoryRowTooLarge) | |||
| 201 | + | } | |||
| 202 | + | checkHistoryAccounting(t, h) | |||
| 203 | + | } | |||
| 204 | + | for i := range 60 { | |||
| 205 | + | appendAt(i) | |||
| 206 | + | } | |||
| 207 | + | shrunk := HistoryConfig{MaxRows: 5, MaxBytes: tc.cfg.MaxBytes, ChunkRows: 2} | |||
| 208 | + | require.NoError(t, h.SetLimits(shrunk)) | |||
| 209 | + | checkHistoryAccounting(t, h) | |||
| 210 | + | for i := 60; i < 75; i++ { | |||
| 211 | + | appendAt(i) | |||
| 212 | + | } | |||
| 213 | + | }) | |||
| 128 | } | 214 | } | ||
| 129 | - | return rows | |||
| 130 | } | 215 | } | ||
| 131 | 216 | ||||
| 132 | // rowIDSet must accept and reject exactly what a plain map would, including | 217 | // rowIDSet must accept and reject exactly what a plain map would, including | ||
| @@ -192,3 +277,38 @@ func TestRestoreAcceptsOutOfOrderDistinctRowIDsAcrossBlobs(t *testing.T) { | |||||
| 192 | _, err = HistoryFromBlobs(HistoryConfig{MaxRows: 16, ChunkRows: 2}, [][]byte{sealed[1], sealed[0], sealed[1]}, tail) | 277 | _, err = HistoryFromBlobs(HistoryConfig{MaxRows: 16, ChunkRows: 2}, [][]byte{sealed[1], sealed[0], sealed[1]}, tail) | ||
| 193 | require.Error(t, err) | 278 | require.Error(t, err) | ||
| 194 | } | 279 | } | ||
| 280 | + | ||||
| 281 | + | // A chunk with more than maxRetainedStyleScratch styles must not leave its | |||
| 282 | + | // large scratch map pinned, whether it is the last or an earlier chunk. | |||
| 283 | + | func TestHistoryStyleScratchDroppedAfterHighCardinalityChunk(t *testing.T) { | |||
| 284 | + | row := make([]renderer.Cell, 1100) | |||
| 285 | + | for x := range row { | |||
| 286 | + | row[x] = renderer.Cell{Rune: 'z', Style: renderer.Style{HasForegroundRGB: true, ForegroundRGB: renderer.RGB{R: uint8(x), G: uint8(x >> 8), B: 1}}} | |||
| 287 | + | } | |||
| 288 | + | h := NewHistory(HistoryConfig{MaxRows: 100, MaxBytes: 1 << 30, ChunkRows: 4}) | |||
| 289 | + | for range 4 { // seals one high-cardinality chunk | |||
| 290 | + | require.NoError(t, h.Append(row, LineBound{End: len(row)})) | |||
| 291 | + | } | |||
| 292 | + | require.LessOrEqual(t, len(h.styleRowScratch), maxRetainedStyleScratch) | |||
| 293 | + | require.LessOrEqual(t, len(h.tailPageStyles), maxRetainedStyleScratch+1) | |||
| 294 | + | for range 3 { | |||
| 295 | + | require.NoError(t, h.Append(historyRow("abcd"), LineBound{End: 4})) | |||
| 296 | + | } | |||
| 297 | + | _ = h.View() | |||
| 298 | + | require.LessOrEqual(t, len(h.styleRowScratch), maxRetainedStyleScratch) | |||
| 299 | + | } | |||
| 300 | + | ||||
| 301 | + | // Early false returns from validation must not leak collected IDs into a later | |||
| 302 | + | // commit. | |||
| 303 | + | func TestRowIDSetBeginDiscardsUncommittedIDs(t *testing.T) { | |||
| 304 | + | var set rowIDSet | |||
| 305 | + | set.begin(0) | |||
| 306 | + | set.collect(1) | |||
| 307 | + | set.collect(2) // abandoned without commit, as on an early invalid return | |||
| 308 | + | set.begin(0) | |||
| 309 | + | set.collect(2) | |||
| 310 | + | require.True(t, set.commit()) | |||
| 311 | + | set.begin(0) | |||
| 312 | + | set.collect(1) | |||
| 313 | + | require.True(t, set.commit()) | |||
| 314 | + | } | |||
M snapshot.go +1 -0
| @@ -179,6 +179,7 @@ func validateRestoredHistoryView(view HistoryView, seen *rowIDSet) bool { | |||
| 179 | 179 | return false | |
| 180 | 180 | } | |
| 181 | 181 | var maxID RowID | |
| 182 | + | seen.begin(0) | |
| 182 | 183 | for _, chunk := range view.chunks { | |
| 183 | 184 | if chunk == nil || chunk.len() != len(chunk.rowIDs) || chunk.len() != len(chunk.bounds) { | |
| 184 | 185 | return false | |
| @@ -179,6 +179,7 @@ func validateRestoredHistoryView(view HistoryView, seen *rowIDSet) bool { | |||||
| 179 | return false | 179 | return false | ||
| 180 | } | 180 | } | ||
| 181 | var maxID RowID | 181 | var maxID RowID | ||
| 182 | + | seen.begin(0) | |||
| 182 | for _, chunk := range view.chunks { | 183 | for _, chunk := range view.chunks { | ||
| 183 | if chunk == nil || chunk.len() != len(chunk.rowIDs) || chunk.len() != len(chunk.bounds) { | 184 | if chunk == nil || chunk.len() != len(chunk.rowIDs) || chunk.len() != len(chunk.bounds) { | ||
| 184 | return false | 185 | return false | ||