Skip to content

feat: cache derived TUI view data to avoid per-frame sorting and cloning#141

Merged
inureyes merged 2 commits into
mainfrom
feature/issue-137-view-data-cache
Apr 6, 2026
Merged

feat: cache derived TUI view data to avoid per-frame sorting and cloning#141
inureyes merged 2 commits into
mainfrom
feature/issue-137-view-data-cache

Conversation

@inureyes

@inureyes inureyes commented Apr 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Add ViewCache (src/view/view_cache.rs) that caches sorted GPU display indices, per-host filtered device subsets (chassis/CPU/memory/storage), and GPU-filtered process lists across frames
  • Integrate cache into FrameRenderer so rendering reads from pre-computed indices instead of re-filtering/re-sorting every frame, with inline fallback when cache is unavailable
  • Wire cache into UiLoop with automatic invalidation on force-clear events (tab change, resize, mode switch) and lazy recomputation keyed by data_version, current_tab, sort_criteria, and gpu_filter_enabled

Test plan

  • All 230 unit tests pass including 14 new cache-specific tests
  • All integration tests pass (container, device, library API, CPU model)
  • cargo clippy -- -D warnings clean
  • cargo fmt --check clean
  • Manual verification: run locally with GPU(s), confirm GPU list sorted correctly after tab/sort changes
  • Manual verification: run in remote mode with multiple hosts, confirm per-host filtering correct after data refreshes
  • Manual verification: toggle GPU filter (f key), confirm process list filters correctly from cache
  • Manual verification: confirm CPU usage reduction compared to post-Snapshot AppState before rendering to shorten the TUI critical section #136 baseline

Closes #137

@inureyes
feat: cache derived TUI view data to avoid per-frame sorting and cloning
c59d093 
Introduce ViewCache that holds pre-computed sorted GPU indices,
per-host filtered device subsets, and GPU-filtered process lists.
Cache entries are keyed by data_version, current_tab, sort_criteria,
and gpu_filter_enabled, and are only recomputed when those inputs
change. This eliminates redundant filtering, sorting, and cloning
on every render pass, particularly beneficial for large remote
deployments with many GPUs and processes.
@inureyes inureyes added type:performance Performance improvements priority:medium Medium priority issue status:review Under review labels Apr 6, 2026
@inureyes
fix: guard against out-of-bounds tab index in GPU filtering paths
398b52e 
Replace direct indexing of snapshot.tabs[current_tab] with
.get() bounds-checked access in both the ViewCache GPU list
builder and the frame renderer fallback path. When current_tab
exceeds tabs.len(), the code now gracefully falls back to
showing all GPUs instead of panicking.

Also add Default impl for ViewCache per Rust conventions.
@inureyes

inureyes commented Apr 6, 2026

Copy link
Copy Markdown
Member Author

Code Review Summary

Reviewed the ViewCache implementation and its integration into the frame renderer and UI loop. The caching strategy is sound: indices into snapshot arrays are cached rather than cloning data, cache keys are keyed by data_version / current_tab / sort_criteria / gpu_filter_enabled, and cache invalidation on force_clear events (tab change, resize, mode switch) is correctly wired through ui_loop.rs.

Findings Addressed

# Severity File Description Status
1 MEDIUM view_cache.rs:179, frame_renderer.rs:204 Potential index out-of-bounds panic: When current_tab >= tabs.len(), the short-circuit is_all_tab check evaluates to false, and the else branch directly indexes tabs[current_tab], which would panic. Replaced with .get() bounds-checked access that falls back to showing all GPUs. Fixed (398b52e)
2 LOW view_cache.rs Missing Default impl: ViewCache::new() produces an all-None struct, which is the natural Default. Added impl Default for ViewCache. Fixed (398b52e)

Findings Reviewed (No Fix Needed)

# Severity Area Description Conclusion
3 OK Cache invalidation Resize events propagate through resize_occurred -> force_clear -> view_cache.invalidate_all() correctly. No issue
4 OK Cache correctness Cache keys include data_version which is bumped by both local_collector and remote_collector on every data update. GPU values cannot change without invalidating the cache. No issue
5 OK Fallback paths All render methods correctly fall back to inline filtering/sorting when cache is None. The Cow::Borrowed pattern for cached process lists avoids unnecessary cloning. No issue
6 LOW view_cache.rs:68 sort_criteria_ordinal uses _ => 0 wildcard for process-only sort variants. If a new GPU-sort variant is added to SortCriteria in the future, it would silently map to Default's ordinal and the cache would serve stale sort order. This is acceptable for now since all current process-only variants intentionally collapse, but worth noting for future maintenance. Deferred

Architecture Assessment

The cache design is well-structured:

  • Three independent cache sections (GPU list, host devices, process filter) with independent invalidation keys prevent unnecessary recomputation when only one dimension changes.
  • Index-based caching for GPU and host-device sections avoids duplicating device data.
  • Process list caching stores Option<Vec<ProcessInfo>> (a clone rather than indices) because the Cow::Borrowed consumption pattern in the renderer requires a contiguous slice. This is a reasonable trade-off.
  • invalidate_all() on force-clear is a conservative but correct approach that ensures stale cache never persists across visual mode transitions.

All 230 tests pass, cargo clippy -- -D warnings is clean, cargo fmt --check is clean.

@inureyes inureyes added status:done Completed and removed status:review Under review labels Apr 6, 2026
@inureyes inureyes merged commit 5c528e1 into main Apr 6, 2026
4 checks passed
@inureyes inureyes deleted the feature/issue-137-view-data-cache branch April 6, 2026 05:25
@inureyes inureyes self-assigned this Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@inureyes inureyes

Labels

priority:medium Medium priority issue status:done Completed type:performance Performance improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cache derived TUI view data to avoid per-frame sorting, filtering, and cloning

1 participant

@inureyes