From efab5855cbd4efab2e3bbc51fad9274b6f7fc5c5 Mon Sep 17 00:00:00 2001 From: deveshctl Date: Sat, 1 Aug 2026 12:21:17 +0530 Subject: [PATCH 1/2] perf(tui): cache file-tree flatten/filter/sort between redraws The file tree recomputed its full flatten->filter->sort pipeline on every call, and the render loop calls it several times per keystroke (cursor bounds, clamp, status bar, and both render passes in split mode). On a large image that redundant per-frame tree walk is the main source of held-key navigation lag. Memoize the result per pane behind a pointer field on the model, keyed on the pipeline inputs (selected layer, filter query, diff-only, sort mode) plus a generation counter for collapse state. A pointer is required because the method has a value receiver and the model is copied by value on every update, so a value field would be written to a throwaway copy. The output is unchanged; only the redundant work is removed. --- CHANGELOG.md | 6 +++ tui/model.go | 78 +++++++++++++++++++++++++++++++ tui/model_test.go | 117 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4daee38..70d83f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- The interactive file tree now caches its flatten/filter/sort result between + redraws, so holding a navigation key or scrolling a large image's tree stays + responsive instead of recomputing the whole tree on every frame. Output is + unchanged; only the redundant per-frame work is removed. + ## [v1.6.0] - 2026-07-28 Eight built-in colour themes, transparent-background mode, and TUI visual diff --git a/tui/model.go b/tui/model.go index b0c0ddd..475a4a6 100644 --- a/tui/model.go +++ b/tui/model.go @@ -166,6 +166,39 @@ type fileSavedMsg struct { var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} +// treeCache memoizes the flatten→filter→sort output of displayTreeFor between +// frames. A single keystroke in split+filter+sort mode drives displayTreeFor up +// to seven times per round-trip (cursor bounds, clamp, status bar, both render +// passes); without a cache each call re-walks the whole FileNode tree. +// +// The cache lives behind a pointer on the model because displayTreeFor has a +// value receiver and the model is copied by value on every Update — a value +// field would be written to a throwaway copy and never survive. All copies of +// the model share one *treeCache; staleness is caught by comparing the stored +// key against the current inputs, not by assuming a copy carried fresh data. +// +// Both panes (focusTree and focusTreeAgg) get an independent slot so split-mode +// rendering, which asks for both trees in the same frame, does not thrash a +// single slot. The generation counter that keys collapse state lives on the +// model (collapsedGen), because toggleCollapsed mutates a map in place and +// returns the same reference — neither map identity nor contents can be +// compared cheaply, so the counter is bumped on every collapse mutation +// instead (see the toggleCollapsed call sites / clear*Collapsed). +type treeCacheSlot struct { + valid bool + layerCursor int + filterQuery string + diffOnly bool + sortMode sortMode + collapsedGen uint64 + files []*image.FileNode +} + +type treeCache struct { + top treeCacheSlot // focusTree + bot treeCacheSlot // focusTreeAgg +} + type model struct { width int height int @@ -239,6 +272,13 @@ type model struct { theme Theme transparentBg bool + // collapsedGen is bumped whenever a collapse map is mutated; it is the + // invalidation key for the displayTreeFor cache (see treeCache). + collapsedGen uint64 + // treeCache memoizes displayTreeFor across frames. Behind a pointer so the + // value-receiver method can write through it and all model copies share it. + treeCache *treeCache + fetchCtx context.Context fetchCancel context.CancelFunc } @@ -283,6 +323,7 @@ func NewModel(cfg Config) model { noCache: cfg.NoCache, theme: themeFor(cfg.Theme), transparentBg: cfg.TransparentBg, + treeCache: &treeCache{}, fetchCtx: ctx, fetchCancel: cancel, } @@ -1067,6 +1108,7 @@ func (m model) tryOpenSelectedFile() (tea.Model, tea.Cmd) { } else { m.treeCollapsed = toggleCollapsed(m.treeCollapsed, f.Path) } + m.collapsedGen++ mp := &m mp.clampCursors() return *mp, nil @@ -1204,7 +1246,41 @@ func (m model) collapsedFor(f focus) map[string]bool { // displayTreeFor flattens, filters, and sorts the tree visible in the given // pane. The same composition rules (collapse → diff-only → filter → sort) // apply to both panes. +// +// The result is memoized per pane (see treeCache). A cache miss recomputes and +// stores; a hit returns the stored slice untouched. When m.treeCache is nil +// (bare model{} literals in tests) it computes through without caching, so the +// observable output is identical with or without the cache. func (m model) displayTreeFor(f focus) []*image.FileNode { + if m.treeCache == nil { + return m.computeDisplayTreeFor(f) + } + slot := &m.treeCache.top + if f == focusTreeAgg { + slot = &m.treeCache.bot + } + if slot.valid && + slot.layerCursor == m.layerCursor && + slot.filterQuery == m.filterQuery && + slot.diffOnly == m.diffOnly && + slot.sortMode == m.sortMode && + slot.collapsedGen == m.collapsedGen { + return slot.files + } + files := m.computeDisplayTreeFor(f) + *slot = treeCacheSlot{ + valid: true, + layerCursor: m.layerCursor, + filterQuery: m.filterQuery, + diffOnly: m.diffOnly, + sortMode: m.sortMode, + collapsedGen: m.collapsedGen, + files: files, + } + return files +} + +func (m model) computeDisplayTreeFor(f focus) []*image.FileNode { root := m.rootFor(f) var files []*image.FileNode if m.useTreeCollapse() { @@ -1243,10 +1319,12 @@ func (m model) activeTreeFocus() focus { func (m *model) clearTreeCollapsed() { m.treeCollapsed = nil + m.collapsedGen++ } func (m *model) clearAggCollapsed() { m.aggCollapsed = nil + m.collapsedGen++ } func (m *model) resetTreeForLayerChange() { diff --git a/tui/model_test.go b/tui/model_test.go index 83069bf..ef7a072 100644 --- a/tui/model_test.go +++ b/tui/model_test.go @@ -2560,3 +2560,120 @@ func TestPageOnEmptyTreeIsNoop(t *testing.T) { m.moveByPage(1, false) assert.Equal(t, 0, m.treeCursor, "paging an empty tree must not move or panic") } + +// --- displayTreeFor cache invalidation --------------------------------------- +// +// These tests exercise the memoization added to displayTreeFor. Each drives one +// input-mutation path and asserts the returned slice reflects the new state. +// They fail if an invalidation key is missing from the cache — the regression +// class the cache introduces. + +func TestDisplayTreeCacheReflectsDiffOnlyToggle(t *testing.T) { + m := setupModelWithDiffs() + before := len(m.displayTreeFor(focusTree)) // warms the cache + + m.diffOnly = true + filtered := m.displayTreeFor(focusTree) + + assert.Less(t, len(filtered), before, "diff-only must drop unchanged files even after the cache is warm") + for _, f := range filtered { + assert.NotEqual(t, image.Unchanged, f.DiffType) + } +} + +func TestDisplayTreeCacheReflectsFilterQuery(t *testing.T) { + m := setupModelWithDiffs() + all := m.displayTreeFor(focusTree) // warms the cache + require.NotEmpty(t, all) + + m.filterQuery = "nginx" + filtered := m.displayTreeFor(focusTree) + + assert.NotEmpty(t, filtered) + assert.Less(t, len(filtered), len(all), "a filter query must narrow the cached slice") + for _, f := range filtered { + assert.Contains(t, strings.ToLower(f.Path), "nginx") + } +} + +func TestDisplayTreeCacheReflectsSortMode(t *testing.T) { + m := setupModelWithDiffs() + unsorted := m.displayTreeFor(focusTree) // warms the cache + require.NotEmpty(t, unsorted) + + m.sortMode = sortDesc + sorted := m.displayTreeFor(focusTree) + + // Sorting by size descending must yield non-increasing effective sizes. + for i := 1; i < len(sorted); i++ { + assert.GreaterOrEqual(t, nodeEffectiveSize(sorted[i-1]), nodeEffectiveSize(sorted[i]), + "sortDesc must return files by descending effective size, not a stale unsorted slice") + } +} + +func TestDisplayTreeCacheReflectsLayerChange(t *testing.T) { + m := setupModelWithDiffs() + m.layerCursor = 0 + base := m.displayTreeFor(focusTree) // warms the cache for layer 0 + + m.layerCursor = 1 + next := m.displayTreeFor(focusTree) + + // The two layers have different tree shapes; a stale cache would return the + // layer-0 slice for layer 1. + assert.NotEqual(t, base, next, "changing the selected layer must recompute the tree") +} + +func TestDisplayTreeCacheReflectsCollapseToggle(t *testing.T) { + m := setupModel() + m.focus = focusTree + expanded := m.displayTreeFor(focusTree) // warms the cache + require.NotEmpty(t, expanded) + + // Collapse the first directory in the visible list. + var dirPath string + for _, f := range expanded { + if f.IsDir { + dirPath = f.Path + break + } + } + require.NotEmpty(t, dirPath, "fixture must contain at least one directory") + + m.treeCollapsed = toggleCollapsed(m.treeCollapsed, dirPath) + m.collapsedGen++ + collapsed := m.displayTreeFor(focusTree) + + assert.Less(t, len(collapsed), len(expanded), + "collapsing a directory must hide its descendants even after the cache is warm") +} + +func TestDisplayTreeCachePanesAreIndependent(t *testing.T) { + m := setupModelWithDiffs() + m.aggregated = true + + // Rendering split mode asks for both panes in one frame; the two cache + // slots must not clobber each other. + top := m.displayTreeFor(focusTree) + bot := m.displayTreeFor(focusTreeAgg) + topAgain := m.displayTreeFor(focusTree) + botAgain := m.displayTreeFor(focusTreeAgg) + + require.NotEmpty(t, top) + require.NotEmpty(t, bot) + assert.Equal(t, &top[0], &topAgain[0], "the top pane slot must survive a bot-pane lookup in the same frame") + assert.Equal(t, &bot[0], &botAgain[0], "the bot pane slot must survive a top-pane lookup in the same frame") + assert.NotEqual(t, &top[0], &bot[0], "the two panes must not share one cache slot") +} + +func TestDisplayTreeCacheHitReturnsIdenticalSlice(t *testing.T) { + m := setupModelWithDiffs() + first := m.displayTreeFor(focusTree) + second := m.displayTreeFor(focusTree) + + // A warm hit with unchanged inputs should return the very same backing + // slice, not recompute a fresh one. + require.NotEmpty(t, first) + assert.Equal(t, &first[0], &second[0], "an unchanged repeat lookup should return the cached slice") +} + From d6bbab0076caa0071c495dd481b2e51066f0f829 Mon Sep 17 00:00:00 2001 From: deveshctl Date: Sun, 2 Aug 2026 10:05:02 +0530 Subject: [PATCH 2/2] perf(tui): key the tree cache on analysis identity The displayTreeFor memoization keyed on layerCursor, filters, sort, and collapse state but not on the analysis itself. If m.analysis were ever replaced while those keys stayed put, a warm cache slot would return the previous analysis's file nodes. Bump an analysisGen counter when m.analysis is assigned and include it in the cache key so a replaced analysis forces a recompute. Dormant today (analysis is set once) but removes the latent staleness trap. --- CHANGELOG.md | 5 +++-- tui/model.go | 13 ++++++++++++- tui/model_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70d83f6..86a059f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - The interactive file tree now caches its flatten/filter/sort result between redraws, so holding a navigation key or scrolling a large image's tree stays - responsive instead of recomputing the whole tree on every frame. Output is - unchanged; only the redundant per-frame work is removed. + responsive instead of recomputing the whole tree on every frame. The cache + refreshes whenever the selected layer, filter, sort, or collapse state + changes. Output is unchanged; only the redundant per-frame work is removed. ## [v1.6.0] - 2026-07-28 diff --git a/tui/model.go b/tui/model.go index 475a4a6..90f2b05 100644 --- a/tui/model.go +++ b/tui/model.go @@ -191,6 +191,7 @@ type treeCacheSlot struct { diffOnly bool sortMode sortMode collapsedGen uint64 + analysisGen uint64 files []*image.FileNode } @@ -275,6 +276,13 @@ type model struct { // collapsedGen is bumped whenever a collapse map is mutated; it is the // invalidation key for the displayTreeFor cache (see treeCache). collapsedGen uint64 + // analysisGen is bumped whenever m.analysis is replaced. It keys the + // displayTreeFor cache against the analysis identity: a valid slot whose + // layerCursor/filters are unchanged would otherwise return the previous + // analysis's FileNode slice after a re-analysis. Today analysis is set + // exactly once, so this only ever reaches 1 — it is future-proofing, not a + // live fix, and costs one comparison per lookup. + analysisGen uint64 // treeCache memoizes displayTreeFor across frames. Behind a pointer so the // value-receiver method can write through it and all model copies share it. treeCache *treeCache @@ -430,6 +438,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.state = stateReady m.analysis = msg.analysis + m.analysisGen++ m.efficiency = image.EfficiencyFromAnalysis(msg.analysis) if src, ok := m.resolver.(image.ExtractorSource); ok { m.extractor = src.NewExtractor() @@ -1264,7 +1273,8 @@ func (m model) displayTreeFor(f focus) []*image.FileNode { slot.filterQuery == m.filterQuery && slot.diffOnly == m.diffOnly && slot.sortMode == m.sortMode && - slot.collapsedGen == m.collapsedGen { + slot.collapsedGen == m.collapsedGen && + slot.analysisGen == m.analysisGen { return slot.files } files := m.computeDisplayTreeFor(f) @@ -1275,6 +1285,7 @@ func (m model) displayTreeFor(f focus) []*image.FileNode { diffOnly: m.diffOnly, sortMode: m.sortMode, collapsedGen: m.collapsedGen, + analysisGen: m.analysisGen, files: files, } return files diff --git a/tui/model_test.go b/tui/model_test.go index ef7a072..edf00d6 100644 --- a/tui/model_test.go +++ b/tui/model_test.go @@ -2677,3 +2677,30 @@ func TestDisplayTreeCacheHitReturnsIdenticalSlice(t *testing.T) { assert.Equal(t, &first[0], &second[0], "an unchanged repeat lookup should return the cached slice") } +func TestDisplayTreeCacheReflectsReanalysis(t *testing.T) { + m := setupModel() + m.focus = focusTree + m.layerCursor = 0 + before := m.displayTreeFor(focusTree) // warms the cache for the first analysis + require.NotEmpty(t, before) + + // A second analysis replaces m.analysis while layerCursor and every filter + // stay put. Without the analysisGen key the warm slot would return the old + // analysis's nodes; the gen bump in the analysisMsg handler must force a + // recompute against the new tree. + other := testAnalysis() + root := other.StackedTrees[0].Root + root.AddChild(&image.FileNode{Name: "REANALYZED", Path: "/REANALYZED", Size: 1}) + m = send(m, analysisMsg{analysis: other}) + + after := m.displayTreeFor(focusTree) + var found bool + for _, f := range after { + if f.Path == "/REANALYZED" { + found = true + break + } + } + assert.True(t, found, "a replaced analysis must invalidate the warm tree cache") +} +