fix: optimize ConsoleViewModel trimming and LogsViewModel batch dispatch#422
Conversation
📝 WalkthroughWalkthroughThis PR optimizes the UI thread dispatch and buffer management in two ViewModels. ConsoleViewModel now uses a conditional trimming strategy for its capped line buffer, and LogsViewModel batches event entries before dispatching them to the UI, replacing per-item posting. ChangesUI ViewModel Performance Optimizations
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
SysManager/SysManager/ViewModels/ConsoleViewModel.cs (1)
44-44: ⚡ Quick winAnchor the 25% cutoff to
MaxLinesinstead ofLines.Count.Line 44 uses
Lines.Count / 4, which shifts the cutoff as the overflow grows. For a stable “25% of buffer” policy, compare againstMaxLines / 4.Proposed change
- if (excess > Lines.Count / 4) + if (excess > MaxLines / 4)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SysManager/SysManager/ViewModels/ConsoleViewModel.cs` at line 44, The cutoff comparison currently uses the dynamic buffer size (Lines.Count) so the 25% threshold moves as content grows; change the comparison to use the fixed buffer size by replacing the use of Lines.Count with MaxLines (i.e., compare excess > MaxLines / 4) in the method where excess is computed so the “25% of buffer” policy is anchored to MaxLines (look for the conditional using excess and Lines.Count in ConsoleViewModel).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@SysManager/SysManager/ViewModels/ConsoleViewModel.cs`:
- Line 44: The cutoff comparison currently uses the dynamic buffer size
(Lines.Count) so the 25% threshold moves as content grows; change the comparison
to use the fixed buffer size by replacing the use of Lines.Count with MaxLines
(i.e., compare excess > MaxLines / 4) in the method where excess is computed so
the “25% of buffer” policy is anchored to MaxLines (look for the conditional
using excess and Lines.Count in ConsoleViewModel).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b68c255-3801-4ce8-a904-e15777846027
📒 Files selected for processing (3)
CHANGELOG.mdSysManager/SysManager/ViewModels/ConsoleViewModel.csSysManager/SysManager/ViewModels/LogsViewModel.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & unit tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (3)
CHANGELOG.md (2)
9-10: LGTM!
11-17: LGTM!SysManager/SysManager/ViewModels/LogsViewModel.cs (1)
135-169: LGTM!
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
## Summary **Batch 4** of the QA bug fix series. ### Issue #400 — Missing error handling for JSON parsing **SpeedTestService.RunOoklaAsync:** - \JsonDocument.Parse(stdout)\ now wrapped in try-catch for \JsonException\ - \GetProperty()\ calls wrapped for \KeyNotFoundException\ - Both re-thrown as \InvalidOperationException\ with descriptive messages - \JsonDocument\ properly disposed via try/finally **WingetService.ParseUpgradeTable** — verified already safe: - \Slice\ helper uses \Math.Min(end, line.Length)\ for bounds checking - Lines shorter than expected are skipped via \if (line.Length < idxAvailable) continue\ **DuplicateFileService.ComputePartialHash** — verified already safe: - Uses \�uffer.AsSpan(0, totalRead)\ to hash only bytes actually read - Loop correctly handles files smaller than 4 KB Closes #400 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
…tch (#422) Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
Summary
Resolves two performance LOWs from the full code review.
Changes
ConsoleViewModel O(n^2) trimming
LogsViewModel batch dispatch
Testing
Summary by CodeRabbit