From 2da9a184661c160b9d887c754941148ceea05276 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 4 Dec 2025 02:41:33 +0000 Subject: [PATCH] Remove analysis documentation files Removed 5 analysis .md files from repository root: - ALLOCATION_CALL_FLOW.md - ALLOCATION_DOCS_README.md - ALLOCATION_INVESTIGATION_SUMMARY.md - HEAP_ALLOCATION_ANALYSIS.md - OPTIMIZATION_RECOMMENDATIONS.md These were temporary analysis documents used during investigation. The key findings and optimizations are captured in the PR description. Co-authored-by: tig <585482+tig@users.noreply.github.com> --- ALLOCATION_CALL_FLOW.md | 363 ------------------ ALLOCATION_DOCS_README.md | 279 -------------- ALLOCATION_INVESTIGATION_SUMMARY.md | 412 --------------------- HEAP_ALLOCATION_ANALYSIS.md | 243 ------------- OPTIMIZATION_RECOMMENDATIONS.md | 546 ---------------------------- 5 files changed, 1843 deletions(-) delete mode 100644 ALLOCATION_CALL_FLOW.md delete mode 100644 ALLOCATION_DOCS_README.md delete mode 100644 ALLOCATION_INVESTIGATION_SUMMARY.md delete mode 100644 HEAP_ALLOCATION_ANALYSIS.md delete mode 100644 OPTIMIZATION_RECOMMENDATIONS.md diff --git a/ALLOCATION_CALL_FLOW.md b/ALLOCATION_CALL_FLOW.md deleted file mode 100644 index 73399b0d8..000000000 --- a/ALLOCATION_CALL_FLOW.md +++ /dev/null @@ -1,363 +0,0 @@ -# Heap Allocation Call Flow Analysis - -## Call Flow Diagram for Progress Bar Scenario - -This document traces the allocation chain from a user interaction down to the heap allocations. - -### High-Level Flow - -``` -User Action (Progress Bar Update) - ↓ -ProgressBar.Fraction = value - ↓ -SetNeedsDraw() - ↓ -Application Main Loop (10-60 Hz) - ↓ -View.OnDrawingContent() / View.DrawContentComplete() - ↓ -TextFormatter.Draw() - ↓ -**ALLOCATION HOTSPOT #1** -GraphemeHelper.GetGraphemes(strings).ToArray() - ↓ -string[] allocated on heap (Gen0) -``` - -### Detailed Call Stack with Line Numbers - -#### 1. Progress Bar Update Path - -``` -Examples/UICatalog/Scenarios/Progress.cs:46 - Application.Invoke(() => systemTimerDemo.Pulse()) - ↓ -Terminal.Gui/Views/ProgressBar.cs:~50 (Pulse method) - Fraction = newValue - ↓ -Terminal.Gui/Views/ProgressBar.cs:~35 - set Fraction { _fraction = value; SetNeedsDraw(); } - ↓ -[View Framework schedules redraw] - ↓ -Terminal.Gui/Views/ProgressBar.cs:135 - OnDrawingContent() - ↓ -[Draws progress bar using Driver.AddStr, etc.] -[Any text on the progress bar view triggers...] - ↓ -Terminal.Gui/ViewBase/View.Drawing.cs:450 - TextFormatter?.Draw(driver, drawRect, normalColor, hotColor) - ↓ -Terminal.Gui/Text/TextFormatter.cs:78-126 - List linesFormatted = GetLines() - foreach line in linesFormatted: - string[] graphemes = GraphemeHelper.GetGraphemes(strings).ToArray() // LINE 126 - ↓ - **ALLOCATION #1: string[] array allocated (size = grapheme count)** - ↓ - [Each grapheme is then drawn to screen] -``` - -#### 2. Border/LineCanvas Update Path - -``` -View with Border - ↓ -Terminal.Gui/ViewBase/Adornment/Border.cs:~500 - OnDrawingContent() - ↓ -[Border needs to draw lines] - ↓ -Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs:210-234 - GetMap(Rectangle inArea) - ↓ - for y in area.Height: - for x in area.Width: - IntersectionDefinition[] intersects = _lines // LINES 219-222 - .Select(l => l.Intersects(x, y)) - .OfType() - .ToArray() - ↓ - **ALLOCATION #2: IntersectionDefinition[] allocated per pixel** - ↓ - [80x24 border = 1,920 allocations] - [100x30 dialog = 2,600 allocations] -``` - -### Allocation Frequency Analysis - -#### Scenario 1: Simple Progress Bar (Default Speed = 100ms) - -**Per Update Cycle:** -1. ProgressBar text (e.g., "Progress: 45%") - - `GetLines()` → splits into 1 line - - `Draw()` line 126 → allocates string[] for ~15 graphemes - - **1 allocation per update** - -2. Percentage label if separate - - Additional 1 allocation - - **Total: 2 allocations per update** - -**Per Second:** -- Update frequency: 10 Hz (every 100ms) -- **20 allocations/second** just for progress display - -**With Border:** -- Border redraws when view redraws -- Typical small progress bar border: ~100 pixels -- **100 additional allocations per update** -- **1,000 allocations/second** for bordered progress bar - -#### Scenario 2: Complex UI (Progress + Clock + Status) - -**Components:** -1. Progress Bar: 20 allocations/second -2. Clock Display (updates every second): 2 allocations/second -3. Status Message: 2 allocations/second (if blinking) -4. Window Border: 260 allocations per redraw - - If progress bar triggers window redraw: 2,600 allocations/second -5. Dialog Borders (if present): 4,800 allocations/second - -**Conservative Estimate:** -- Progress bar alone: 20-120 allocations/second -- With borders: 1,000-3,000 allocations/second -- Full complex UI: **Easily 5,000-10,000 allocations/second** - -### Memory Allocation Types - -#### Type 1: String Arrays (TextFormatter) - -```csharp -// Terminal.Gui/Text/TextFormatter.cs:126 -string[] graphemes = GraphemeHelper.GetGraphemes(strings).ToArray(); -``` - -**Size per allocation:** -- Array header: 24 bytes (x64) -- Per element: 8 bytes (reference) -- Plus: Each string object (grapheme) -- **Typical: 50-500 bytes per allocation** - -#### Type 2: IntersectionDefinition Arrays (LineCanvas) - -```csharp -// Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs:219-222 -IntersectionDefinition[] intersects = _lines - .Select(l => l.Intersects(x, y)) - .OfType() - .ToArray(); -``` - -**Size per allocation:** -- Array header: 24 bytes -- Per IntersectionDefinition: ~32 bytes (struct size) -- Typical line count: 2-6 intersections -- **Typical: 50-200 bytes per allocation** -- **But happens ONCE PER PIXEL!** - -#### Type 3: List (Various TextFormatter methods) - -```csharp -// Terminal.Gui/Text/TextFormatter.cs:1336, 1460, 1726 -List graphemes = GraphemeHelper.GetGraphemes(text).ToList(); -``` - -**Size per allocation:** -- List object: 32 bytes -- Internal array: Variable (resizes as needed) -- **Typical: 100-1,000 bytes per allocation** - -### Garbage Collection Impact - -#### Gen0 Collection Triggers - -Assuming: -- Gen0 threshold: ~16 MB (typical) -- Average allocation: 200 bytes -- Complex UI: 5,000 allocations/second -- **Memory allocated per second: ~1 MB** - -**Result:** -- Gen0 collection approximately every 16 seconds -- With heap fragmentation and other allocations: **Every 5-10 seconds** - -#### GC Pause Impact - -- Gen0 collection pause: 1-5ms (typical) -- At 60 FPS UI: 16.67ms per frame budget -- **GC pause consumes 6-30% of frame budget** -- Result: Frame drops, UI stuttering during GC - -### Optimization Opportunities - -#### 1. Array Pooling (TextFormatter.Draw) - -**Current:** -```csharp -string[] graphemes = GraphemeHelper.GetGraphemes(strings).ToArray(); -``` - -**Optimized:** -```csharp -// Use ArrayPool -string[] graphemes = ArrayPool.Shared.Rent(estimatedSize); -try { - int count = 0; - foreach (var g in GraphemeHelper.GetGraphemes(strings)) { - graphemes[count++] = g; - } - // Use graphemes[0..count] -} finally { - ArrayPool.Shared.Return(graphemes); -} -``` - -**Benefit:** Zero allocations for repeated draws - -#### 2. Span-Based Processing (LineCanvas.GetMap) - -**Current:** -```csharp -IntersectionDefinition[] intersects = _lines - .Select(l => l.Intersects(x, y)) - .OfType() - .ToArray(); -``` - -**Optimized (like GetCellMap):** -```csharp -List intersectionsBufferList = []; // Reuse outside loop -// Inside loop: -intersectionsBufferList.Clear(); -foreach (var line in _lines) { - if (line.Intersects(x, y) is { } intersect) { - intersectionsBufferList.Add(intersect); - } -} -ReadOnlySpan intersects = CollectionsMarshal.AsSpan(intersectionsBufferList); -``` - -**Benefit:** Zero per-pixel allocations (from 1,920+ to 0 per border redraw) - -#### 3. Grapheme Caching - -**Concept:** Cache grapheme arrays for unchanging text - -```csharp -class TextFormatter { - private string? _cachedText; - private string[]? _cachedGraphemes; - - string[] GetGraphemesWithCache(string text) { - if (text == _cachedText && _cachedGraphemes != null) { - return _cachedGraphemes; - } - _cachedText = text; - _cachedGraphemes = GraphemeHelper.GetGraphemes(text).ToArray(); - return _cachedGraphemes; - } -} -``` - -**Benefit:** Zero allocations for static text (labels, buttons) - -### Measurement Tools - -#### 1. BenchmarkDotNet - -Already used in project: -```bash -cd Tests/Benchmarks -dotnet run -c Release --filter "*TextFormatter*" -``` - -Provides: -- Allocation counts -- Memory per operation -- Speed comparisons - -#### 2. dotnet-trace - -```bash -dotnet-trace collect --process-id --providers Microsoft-Windows-DotNETRuntime:0x1:5 -``` - -Captures: -- GC events -- Allocation stacks -- GC pause times - -#### 3. Visual Studio Profiler - -- .NET Object Allocation Tracking -- Shows allocation hot paths -- Call tree with allocation counts - -### Expected Results After Optimization - -#### TextFormatter.Draw Optimization - -**Before:** -- 1 allocation per Draw call -- 10-60 allocations/second for animated content - -**After:** -- 0 allocations (with ArrayPool) -- OR 1 allocation per unique text (with caching) - -**Improvement:** 90-100% reduction in allocations - -#### LineCanvas.GetMap Optimization - -**Before:** -- Width × Height allocations per redraw -- 1,920 allocations for 80×24 border - -**After:** -- 1 allocation per GetMap call (reused buffer) - -**Improvement:** 99.95% reduction (from 1,920 to 1) - -#### Overall Impact - -**Complex UI Scenario:** - -| Metric | Before | After | Improvement | -|--------|--------|-------|-------------| -| Allocations/sec | 5,000-10,000 | 50-100 | 99% | -| Gen0 GC frequency | Every 5-10s | Every 80-160s | 16x reduction | -| Memory pressure | High | Low | Significant | -| Frame drops | Occasional | Rare | Noticeable | -| CPU overhead (GC) | 5-10% | <1% | 10x reduction | - -### Validation Strategy - -1. **Add allocation benchmarks** - - Benchmark Draw method - - Benchmark GetMap method - - Compare before/after - -2. **Run Progress scenario** - - Profile with dotnet-trace - - Measure GC frequency - - Verify allocation reduction - -3. **Stress test** - - Multiple progress bars - - Complex borders - - Animated content - - Measure sustained performance - -### Conclusion - -The allocation call flow analysis confirms: - -1. **TextFormatter.Draw** is a critical path for text-heavy UIs -2. **LineCanvas.GetMap** has severe per-pixel allocation issues -3. **Optimization patterns exist** and are already partially implemented -4. **Expected improvement is 90-99%** allocation reduction -5. **Measurement tools are available** to validate improvements - -The path forward is clear, with existing code patterns (GetCellMap) providing a blueprint for optimization. diff --git a/ALLOCATION_DOCS_README.md b/ALLOCATION_DOCS_README.md deleted file mode 100644 index 5d9c1efb5..000000000 --- a/ALLOCATION_DOCS_README.md +++ /dev/null @@ -1,279 +0,0 @@ -# Heap Allocation Investigation - Document Guide - -This directory contains comprehensive documentation of the heap allocation analysis performed on Terminal.Gui in response to reported performance issues with intermediate allocations. - -## Quick Start - -**If you want to...** - -- 📊 **Understand the problem quickly** → Read [ALLOCATION_INVESTIGATION_SUMMARY.md](ALLOCATION_INVESTIGATION_SUMMARY.md) -- 🔍 **See detailed technical analysis** → Read [HEAP_ALLOCATION_ANALYSIS.md](HEAP_ALLOCATION_ANALYSIS.md) -- 🛠️ **Implement the fixes** → Read [OPTIMIZATION_RECOMMENDATIONS.md](OPTIMIZATION_RECOMMENDATIONS.md) -- 📈 **Understand call flows** → Read [ALLOCATION_CALL_FLOW.md](ALLOCATION_CALL_FLOW.md) - -## Document Overview - -### 1. [ALLOCATION_INVESTIGATION_SUMMARY.md](ALLOCATION_INVESTIGATION_SUMMARY.md) -**Type:** Executive Summary -**Audience:** Project maintainers, decision makers -**Length:** ~10 pages - -**Contents:** -- TL;DR with key findings -- Critical allocation hotspots (2 main issues) -- Real-world impact quantification -- Risk assessment -- Recommended next steps -- Decision point for maintainers - -**Read this if:** You need to understand the issue quickly and decide on next steps - ---- - -### 2. [HEAP_ALLOCATION_ANALYSIS.md](HEAP_ALLOCATION_ANALYSIS.md) -**Type:** Technical Analysis -**Audience:** Developers, performance engineers -**Length:** ~15 pages - -**Contents:** -- Complete list of 9 allocation hotspots with line numbers -- Root cause analysis for each issue -- Detailed performance impact estimates -- Memory allocation type breakdown -- GC impact analysis -- Comparison to v2_develop branch - -**Read this if:** You need complete technical details and want to understand why allocations happen - ---- - -### 3. [ALLOCATION_CALL_FLOW.md](ALLOCATION_CALL_FLOW.md) -**Type:** Call Flow Analysis -**Audience:** Developers working on fixes -**Length:** ~12 pages - -**Contents:** -- Detailed call stacks from user action to allocation -- Frequency analysis per scenario -- Allocation size calculations -- GC trigger estimations -- Code examples showing allocation points -- Measurement tool recommendations - -**Read this if:** You're implementing fixes and need to understand the execution path - ---- - -### 4. [OPTIMIZATION_RECOMMENDATIONS.md](OPTIMIZATION_RECOMMENDATIONS.md) -**Type:** Implementation Roadmap -**Audience:** Developers implementing solutions -**Length:** ~18 pages - -**Contents:** -- Prioritized fix list (P0, P1, P2, P3) -- Concrete code solutions with examples -- 4-phase implementation roadmap (2-3 weeks) -- Testing strategy and benchmarks -- Success metrics and validation approach -- Breaking change considerations -- Risk assessment per change - -**Read this if:** You're ready to implement optimizations and need detailed guidance - ---- - -## Reading Order Recommendations - -### For Decision Makers -1. ALLOCATION_INVESTIGATION_SUMMARY.md (complete) -2. OPTIMIZATION_RECOMMENDATIONS.md (section: Priority Ranking) - -**Time:** 15-20 minutes -**Goal:** Understand issue and approve work - -### For Developers Implementing Fixes -1. ALLOCATION_INVESTIGATION_SUMMARY.md (complete) -2. HEAP_ALLOCATION_ANALYSIS.md (sections: Critical Allocation Hotspots, Root Cause) -3. OPTIMIZATION_RECOMMENDATIONS.md (complete) -4. ALLOCATION_CALL_FLOW.md (as reference during implementation) - -**Time:** 1-2 hours -**Goal:** Full understanding before coding - -### For Performance Engineers -1. HEAP_ALLOCATION_ANALYSIS.md (complete) -2. ALLOCATION_CALL_FLOW.md (complete) -3. OPTIMIZATION_RECOMMENDATIONS.md (sections: Testing Strategy, Monitoring) - -**Time:** 2-3 hours -**Goal:** Deep understanding for optimization and benchmarking - -### For Code Reviewers -1. ALLOCATION_INVESTIGATION_SUMMARY.md (complete) -2. OPTIMIZATION_RECOMMENDATIONS.md (sections: Implementation Roadmap, Risk Assessment) -3. HEAP_ALLOCATION_ANALYSIS.md (as reference for context) - -**Time:** 30-45 minutes -**Goal:** Understand changes being reviewed - ---- - -## Key Findings at a Glance - -### 🔴 Critical Issues (P0) - -1. **LineCanvas.GetMap()** - `Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs:219-222` - - Per-pixel array allocation in nested loop - - Impact: 1,920+ allocations per border redraw - - Fix: Apply existing GetCellMap() pattern - - Effort: 4-8 hours - -2. **TextFormatter.Draw()** - `Terminal.Gui/Text/TextFormatter.cs:126` - - Array allocation on every draw call - - Impact: 10-60+ allocations per second - - Fix: Use ArrayPool - - Effort: 1-2 days - -### 📊 Performance Impact - -**Current State:** -- Progress bar demo: 3,000-5,000 allocations/second -- Gen0 GC: Every 5-10 seconds -- Result: Visible frame drops - -**After Fixes:** -- Allocations: 50-100/second (98% reduction) -- Gen0 GC: Every 80-160 seconds (16× improvement) -- Result: Smooth performance - -### ✅ Solution Confidence: HIGH - -- Proven patterns (GetCellMap already works) -- Standard .NET tools (ArrayPool, Span) -- Low implementation risk -- Clear testing strategy - ---- - -## Investigation Methodology - -This analysis was conducted through: - -1. **Static Code Analysis** - - Searched for `.ToArray()` and `.ToList()` patterns - - Identified allocation sites with line numbers - - Traced call stacks to understand frequency - -2. **Frequency Analysis** - - Examined Progress demo code (100ms update interval) - - Analyzed ProgressBar.Fraction property (calls SetNeedsDraw) - - Counted allocations per update cycle - -3. **Memory Impact Calculation** - - Estimated allocation sizes per operation - - Calculated allocations per second for scenarios - - Projected GC behavior based on allocation rate - -4. **Solution Research** - - Found existing optimizations (GetCellMap) - - Identified proven patterns (ArrayPool usage) - - Validated approach feasibility - -5. **Documentation** - - Created comprehensive analysis documents - - Provided actionable recommendations - - Included code examples and roadmap - ---- - -## Next Steps - -### Immediate Actions (This Week) - -1. **Review documents** with team -2. **Approve Phase 1** work if agreed -3. **Assign developer** for LineCanvas.GetMap() fix -4. **Set up benchmarks** to measure current state - -### Short Term (Next 2 Weeks) - -1. **Implement P0 fixes** (LineCanvas + TextFormatter) -2. **Validate improvements** with benchmarks -3. **Run Progress demo** profiling - -### Medium Term (Next Month) - -1. **Complete optimization roadmap** (all P1-P3 items) -2. **Add comprehensive tests** -3. **Update performance documentation** - ---- - -## Questions or Feedback - -**For technical questions about the analysis:** -- Review the specific document section -- Check OPTIMIZATION_RECOMMENDATIONS.md for code examples -- Consult ALLOCATION_CALL_FLOW.md for execution details - -**For implementation questions:** -- Start with OPTIMIZATION_RECOMMENDATIONS.md -- Reference code examples provided -- Review existing GetCellMap() implementation as template - -**For performance measurement:** -- See ALLOCATION_CALL_FLOW.md (Measurement Tools section) -- See OPTIMIZATION_RECOMMENDATIONS.md (Testing Strategy section) -- Benchmark infrastructure already exists in Tests/Benchmarks - ---- - -## Related Files in Repository - -**Source Code:** -- `Terminal.Gui/Text/TextFormatter.cs` - Main text rendering -- `Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs` - Border/line rendering -- `Terminal.Gui/Drawing/GraphemeHelper.cs` - Text element enumeration -- `Terminal.Gui/Drawing/Cell.cs` - Cell validation - -**Tests:** -- `Tests/Benchmarks/Text/TextFormatter/` - Existing benchmarks -- `Tests/UnitTests/` - Unit test infrastructure -- `Tests/UnitTestsParallelizable/` - Parallel test infrastructure - -**Examples:** -- `Examples/UICatalog/Scenarios/Progress.cs` - Progress demo (mentioned in issue) - ---- - -## Document Maintenance - -These documents were created on **December 3, 2025** as part of investigating the intermediate heap allocation issue. - -**Status:** Investigation Complete ✅ - -**Action Required:** Decision on implementing optimizations - -**Owner:** Awaiting assignment based on maintainer decision - ---- - -## Summary - -Four comprehensive documents totaling ~55 pages provide: -- Complete problem analysis -- Quantified performance impact -- Concrete solutions with code examples -- Implementation roadmap with timeline -- Testing and validation strategy - -**The issue is confirmed, significant, and solvable.** Documentation provides everything needed to proceed with confidence. - ---- - -**Document Navigation:** -- [← Back to Root](/) -- [Executive Summary →](ALLOCATION_INVESTIGATION_SUMMARY.md) -- [Technical Analysis →](HEAP_ALLOCATION_ANALYSIS.md) -- [Call Flow Analysis →](ALLOCATION_CALL_FLOW.md) -- [Implementation Guide →](OPTIMIZATION_RECOMMENDATIONS.md) diff --git a/ALLOCATION_INVESTIGATION_SUMMARY.md b/ALLOCATION_INVESTIGATION_SUMMARY.md deleted file mode 100644 index 115c9723b..000000000 --- a/ALLOCATION_INVESTIGATION_SUMMARY.md +++ /dev/null @@ -1,412 +0,0 @@ -# Heap Allocation Investigation - Executive Summary - -**Investigation Date:** December 3, 2025 -**Investigator:** GitHub Copilot Agent -**Issue Reference:** Intermediate heap allocations in TextFormatter and LineCanvas - ---- - -## TL;DR - -✅ **Issue Confirmed:** The heap allocation problem is **REAL and SIGNIFICANT** - -🔴 **Severity:** **CRITICAL** for animated UIs, progress bars, and border-heavy layouts - -📊 **Impact:** 1,000-10,000 allocations per second in typical scenarios - -✅ **Solution:** Clear path forward using ArrayPool, Span, and buffer reuse - -⏱️ **Timeline:** 2-3 weeks for complete fix, quick wins available immediately - ---- - -## What We Found - -### Critical Allocation Hotspots - -#### 1. LineCanvas.GetMap() - **MOST CRITICAL** - -**Location:** `Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs:219-222` - -```csharp -// Allocates array PER PIXEL in nested loop -IntersectionDefinition[] intersects = _lines - .Select(l => l.Intersects(x, y)) - .OfType() - .ToArray(); // ❌ Inside double loop! -``` - -**Impact:** -- 80×24 window border: **1,920 allocations per redraw** -- 100×30 dialog: **4,800 allocations per redraw** -- Quadratic allocation pattern (O(width × height)) - -**Fix Complexity:** ⭐ Easy (pattern already exists in same file) -**Impact:** ⭐⭐⭐⭐⭐ Massive (99%+ reduction) - ---- - -#### 2. TextFormatter.Draw() - **VERY CRITICAL** - -**Location:** `Terminal.Gui/Text/TextFormatter.cs:126` - -```csharp -// Allocates array on every draw call -string[] graphemes = GraphemeHelper.GetGraphemes(strings).ToArray(); -``` - -**Impact:** -- Called 10-60+ times per second for animated content -- Every progress bar update -- Every text view redraw -- Compounds with multiple views - -**Fix Complexity:** ⭐⭐⭐ Medium (ArrayPool implementation) -**Impact:** ⭐⭐⭐⭐⭐ Massive (90-100% reduction) - ---- - -### Additional Allocation Points - -**TextFormatter.cs:** 7 more allocation sites in helper methods -- Lines: 934, 1336, 1407, 1460, 1726, 2191, 2300 - -**Cell.cs:** Validation allocates unnecessarily -- Line: 30 - -**Total Identified:** 9 distinct allocation hotspots - ---- - -## Real-World Impact - -### Progress Bar Demo (Referenced in Issue) - -**Scenario:** Progress bar updating every 100ms - -| Component | Allocations/Update | Frequency | Allocations/Sec | -|-----------|-------------------|-----------|-----------------| -| Progress bar text | 1-2 | 10 Hz | 10-20 | -| Border (if present) | 100-260 | 10 Hz | 1,000-2,600 | -| Window redraw | 260 | 10 Hz | 2,600 | -| **Total** | | | **3,610-5,220** | - -**Result:** ~4,000 allocations per second for a simple progress bar! - -### Complex UI (Progress + Time + Status) - -**Scenario:** Dashboard with multiple updating elements - -| Component | Allocations/Sec | -|-----------|-----------------| -| Progress bars (2×) | 40-5,200 | -| Clock display | 2-4 | -| Status messages | 2-20 | -| Borders/chrome | 2,600-4,800 | -| **Total** | **5,000-10,000** | - -**Result:** Gen0 GC every 5-10 seconds, causing frame drops - ---- - -## Memory Pressure Analysis - -### Allocation Breakdown - -``` -Per Progress Bar Update (100ms): -├─ Text: 200 bytes (1 string[] allocation) -├─ Border: 20 KB (1,920 array allocations) -└─ Total: ~20 KB per update - -Per Second (10 updates): -├─ 200 KB from progress bars -├─ Additional UI updates: ~800 KB -└─ Total: ~1 MB/second allocation rate -``` - -### GC Impact - -**Assumptions:** -- Gen0 threshold: ~16 MB -- Allocation rate: 1 MB/sec -- Result: Gen0 collection every 10-16 seconds - -**Reality:** -- With heap fragmentation: Every 5-10 seconds -- Gen0 pause: 1-5ms per collection -- At 60 FPS: Consumes 6-30% of frame budget -- Result: **Visible stuttering during GC** - ---- - -## Why v2 Branch Is Worse - -The issue mentions v2_develop has increased allocations, particularly from LineCanvas. - -**Likely Causes:** -1. More border/line usage in v2 UI framework -2. GetMap() called more frequently -3. Per-pixel allocation multiplied by increased usage - -**Confirmation:** LineCanvas.GetMap() has severe per-pixel allocation issue - ---- - -## Evidence Supporting Findings - -### 1. Code Analysis - -✅ Direct observation of `.ToArray()` in hot paths -✅ Nested loops with allocations inside -✅ Called from frequently-executed code paths - -### 2. Call Stack Tracing - -✅ Traced from ProgressBar.Fraction → TextFormatter.Draw() -✅ Traced from Border.OnDrawingContent() → LineCanvas.GetMap() -✅ Documented with exact line numbers - -### 3. Frequency Analysis - -✅ Progress demo updates 10 Hz (confirmed in code) -✅ ProgressBar.Fraction calls SetNeedsDraw() (confirmed) -✅ Draw methods called on every redraw (confirmed) - -### 4. Existing Optimizations - -✅ LineCanvas.GetCellMap() already uses buffer reuse pattern -✅ Proves solution is viable and working -✅ Just needs to be applied to GetMap() - ---- - -## Recommended Solution - -### Immediate (Phase 1): Quick Wins - -**1. Fix LineCanvas.GetMap()** - 4-8 hours - -Apply the existing GetCellMap() pattern: -- Reuse buffer list -- Use CollectionsMarshal.AsSpan() -- **Impact:** 99%+ reduction (1,920 → 1 allocation per redraw) - -**2. Add GraphemeHelper.GetGraphemeCount()** - 1-2 hours - -For validation without allocation: -- **Impact:** Zero allocations for Cell.Grapheme validation - -### Short-term (Phase 2): Core Fix - -**3. ArrayPool in TextFormatter.Draw()** - 1-2 days - -Use ArrayPool for grapheme arrays: -- **Impact:** 90-100% reduction in text draw allocations - -**4. Benchmarks & Testing** - 1 day - -Measure and validate improvements: -- Add BenchmarkDotNet tests -- Profile Progress demo -- Confirm allocation reduction - -### Medium-term (Phase 3): Complete Solution - -**5. Update Helper Methods** - 5-7 days - -Add span-based APIs, update all allocation points: -- **Impact:** Complete allocation-free text rendering path - ---- - -## Expected Results - -### Before Optimization - -| Metric | Value | -|--------|-------| -| Allocations/sec (Progress demo) | 3,000-5,000 | -| Gen0 GC frequency | Every 5-10 seconds | -| Memory allocated/sec | ~1 MB | -| Frame drops | Occasional | -| GC pause impact | 5-10% CPU | - -### After Optimization - -| Metric | Value | Improvement | -|--------|-------|-------------| -| Allocations/sec | 50-100 | **98% reduction** | -| Gen0 GC frequency | Every 80-160 sec | **16× less frequent** | -| Memory allocated/sec | <50 KB | **95% reduction** | -| Frame drops | Rare | Significant | -| GC pause impact | <1% CPU | **10× reduction** | - ---- - -## Risk Assessment - -### Implementation Risk: **LOW** ✅ - -- Solutions use proven .NET patterns (ArrayPool, Span) -- Existing code demonstrates viability (GetCellMap) -- Extensive test infrastructure available -- No breaking API changes required - -### Performance Risk: **VERY LOW** ✅ - -- Optimizations only improve performance -- No functional changes -- Backward compatible - -### Maintenance Risk: **LOW** ✅ - -- Standard .NET patterns -- Well-documented solutions -- Clear test coverage - ---- - -## Validation Strategy - -### 1. Benchmarks - -```bash -cd Tests/Benchmarks -dotnet run -c Release --filter "*Allocation*" -``` - -Measure: -- Allocations per operation -- Bytes allocated -- Speed comparison - -### 2. Profiling - -```bash -# Run Progress demo -dotnet run --project Examples/UICatalog - -# Profile with dotnet-trace -dotnet-trace collect --process-id \ - --providers Microsoft-Windows-DotNETRuntime:0x1:5 -``` - -Capture: -- GC events -- Allocation stacks -- Pause times - -### 3. Unit Tests - -Add allocation-aware tests: -```csharp -[Fact] -public void Draw_NoAllocations_WithOptimization() -{ - long before = GC.GetAllocatedBytesForCurrentThread(); - textFormatter.Draw(...); - long after = GC.GetAllocatedBytesForCurrentThread(); - - Assert.True(after - before < 1000); -} -``` - ---- - -## Documentation Provided - -This investigation produced four comprehensive documents: - -### 1. **HEAP_ALLOCATION_ANALYSIS.md** (Main Report) -- Detailed technical analysis -- All 9 allocation hotspots documented -- Root cause analysis -- Performance impact estimation - -### 2. **ALLOCATION_CALL_FLOW.md** (Call Flow) -- Call stack traces with line numbers -- Frequency analysis per scenario -- Allocation type breakdown -- GC impact calculations - -### 3. **OPTIMIZATION_RECOMMENDATIONS.md** (Implementation Guide) -- Prioritized fix list (P0, P1, P2, P3) -- Concrete code solutions -- 4-phase implementation roadmap -- Testing strategy and success metrics - -### 4. **ALLOCATION_INVESTIGATION_SUMMARY.md** (This Document) -- Executive summary -- Key findings and recommendations -- Expected results and risk assessment - ---- - -## Conclusion - -### The Issue Is Real ✅ - -The intermediate heap allocation problem described in the issue is: -- ✅ **Confirmed** through code analysis -- ✅ **Quantified** with specific numbers -- ✅ **Reproducible** in the Progress demo -- ✅ **Significant** in impact - -### The Issue Is Solvable ✅ - -Solutions are: -- ✅ **Clear** and well-documented -- ✅ **Proven** (patterns already exist in codebase) -- ✅ **Low risk** (standard .NET optimizations) -- ✅ **High impact** (90-99% allocation reduction) - -### Recommended Next Steps - -1. **Immediate:** Fix LineCanvas.GetMap() (4-8 hours, massive impact) -2. **This Week:** Add benchmarks to measure current state -3. **Next Week:** Implement TextFormatter.Draw() optimization -4. **This Month:** Complete all optimizations per roadmap - -### Priority Justification - -This issue should be **HIGH PRIORITY** because: -- Affects common scenarios (progress bars, animations, borders) -- Causes visible performance degradation (GC pauses, stuttering) -- Has clear, low-risk solution path -- Provides immediate, measurable improvement - ---- - -## For Project Maintainers - -**Decision Required:** Approve optimization work? - -**If Yes:** -- Review OPTIMIZATION_RECOMMENDATIONS.md for roadmap -- Assign Phase 1 work (LineCanvas + benchmarks) -- Target completion: 2-3 weeks for full optimization - -**If No:** -- Issue can be triaged/prioritized differently -- Documentation remains as reference for future work - ---- - -## Contact & Questions - -This investigation was conducted as requested in the issue to assess the scope and severity of intermediate heap allocations. - -All analysis is based on: -- Direct code inspection -- Static analysis of allocation patterns -- Frequency calculations from code behavior -- Industry-standard optimization patterns - -For questions or clarifications, refer to the detailed documents listed above. - ---- - -**Investigation Complete** ✅ - -The Terminal.Gui codebase has been thoroughly analyzed for heap allocation issues. The findings confirm significant problems with clear solutions. Implementation can proceed with confidence. diff --git a/HEAP_ALLOCATION_ANALYSIS.md b/HEAP_ALLOCATION_ANALYSIS.md deleted file mode 100644 index 5def6aa7d..000000000 --- a/HEAP_ALLOCATION_ANALYSIS.md +++ /dev/null @@ -1,243 +0,0 @@ -# Heap Allocation Analysis for Terminal.Gui - -## Executive Summary - -This document provides a comprehensive analysis of intermediate heap allocations in Terminal.Gui, focusing on the `TextFormatter` and `LineCanvas` classes as reported in the issue. - -## Severity Assessment: **HIGH IMPACT** - -The allocation issues identified are significant performance concerns that affect: -- Every frame redraw in UI scenarios -- Any time-based updates (progress bars, timers, clocks) -- Text rendering operations -- Border and line drawing operations - -## Key Findings - -### 1. TextFormatter Class (`Terminal.Gui/Text/TextFormatter.cs`) - -#### Critical Allocation Hotspots - -**Location: Line 126 (in `Draw` method)** -```csharp -string[] graphemes = GraphemeHelper.GetGraphemes (strings).ToArray (); -``` -- **Frequency**: Every time Draw is called (potentially 60+ times per second during animations) -- **Impact**: Allocates a new string array for every line being drawn -- **Called from**: View.Drawing.cs, Border.cs, TextField.cs, and other UI components - -**Location: Line 934 (in `GetDrawRegion` method)** -```csharp -string [] graphemes = GraphemeHelper.GetGraphemes (strings).ToArray (); -``` -- **Frequency**: Every time region calculation is needed -- **Impact**: Similar allocation for grapheme arrays - -**Additional Allocation Points:** -- Line 1336: `List graphemes = GraphemeHelper.GetGraphemes (text).ToList ();` in `SplitNewLine` -- Line 1407: `string [] graphemes = GraphemeHelper.GetGraphemes (text).ToArray ();` in `ClipOrPad` -- Line 1460: `List graphemes = GraphemeHelper.GetGraphemes (StripCRLF (text)).ToList ();` in `WordWrapText` -- Line 1726: `List graphemes = GraphemeHelper.GetGraphemes (text).ToList ();` in `ClipAndJustify` -- Line 2191: `string [] graphemes = GraphemeHelper.GetGraphemes (text).ToArray ();` in `GetSumMaxCharWidth` -- Line 2300: `string [] graphemes = GraphemeHelper.GetGraphemes (lines [lineIdx]).ToArray ();` in `GetMaxColsForWidth` - -**Total Count**: 9 distinct allocation points in TextFormatter alone - -#### Why This Matters - -The `Draw` method is called: -1. On every frame update for animated content -2. When any view needs to redraw its text -3. During progress bar updates (the example mentioned in the issue) -4. For real-time displays (clocks, status bars) - -With a typical progress bar updating at 10-30 times per second, and potentially multiple text elements on screen, this can result in **hundreds to thousands of allocations per second**. - -### 2. LineCanvas Class (`Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs`) - -#### Critical Allocation Hotspot - -**Location: Lines 219-222 (in `GetMap(Rectangle inArea)` method)** -```csharp -IntersectionDefinition [] intersects = _lines - .Select (l => l.Intersects (x, y)) - .OfType () - .ToArray (); -``` - -- **Frequency**: **Once per pixel in the area** (nested loop over x and y) -- **Impact**: EXTREMELY HIGH - allocates array for every single pixel being evaluated -- **Example**: A 80x24 terminal window border = 1,920 allocations per redraw -- **Example**: A 120x40 dialog with borders = 4,800 allocations per redraw - -#### Good News - -The `GetCellMap()` method (line 162) was already optimized: -```csharp -List intersectionsBufferList = []; -// ... reuses list with Clear() ... -ReadOnlySpan intersects = CollectionsMarshal.AsSpan(intersectionsBufferList); -``` - -This is the **correct pattern** - reusing a buffer and using spans to avoid allocations. - -### 3. Cell Class (`Terminal.Gui/Drawing/Cell.cs`) - -**Location: Line 30** -```csharp -if (GraphemeHelper.GetGraphemes(value).ToArray().Length > 1) -``` - -- **Frequency**: Every time Grapheme property is set -- **Impact**: Moderate - validation code - -### 4. GraphemeHelper Pattern - -The core issue is that `GraphemeHelper.GetGraphemes()` returns an `IEnumerable`, which is then immediately materialized to arrays or lists. This pattern appears throughout the codebase. - -## Root Cause Analysis - -### TextFormatter Allocations - -The fundamental issue is the design pattern: -1. `GetGraphemes()` returns `IEnumerable` (lazy enumeration) -2. Code immediately calls `.ToArray()` or `.ToList()` to materialize it -3. This happens on every draw call, creating garbage - -### LineCanvas Allocations - -The `GetMap(Rectangle inArea)` method has a particularly problematic nested loop structure: -- Outer loop: Y coordinates -- Inner loop: X coordinates -- **Inside inner loop**: LINQ query with `.ToArray()` allocation - -This is a classic O(n²) allocation problem where the allocation count grows quadratically with area size. - -## Performance Impact Estimation - -### TextFormatter in Progress Demo - -Assuming: -- Progress bar updates 20 times/second -- Each update redraws the bar (1 line) and percentage text (1 line) -- Each line calls `Draw()` which allocates an array - -**Result**: 40 array allocations per second, just for the progress bar - -Add a clock display updating once per second, status messages, etc., and we easily reach **hundreds of allocations per second** in a moderately complex UI. - -### LineCanvas in Border Drawing - -A typical dialog window: -- 100x30 character area -- Border needs to evaluate 2×(100+30) = 260 pixels for the border -- Each pixel: 1 array allocation - -**Result**: 260 allocations per border redraw - -If the dialog is redrawn 10 times per second (e.g., with animated content inside), that's **2,600 allocations per second** just for one border. - -## Comparison to v2_develop Branch - -The issue mentions that allocations "increased drastically" on the v2_develop branch, particularly from LineCanvas. This is consistent with the findings: - -1. **GetMap(Rectangle)** method allocates per-pixel -2. If border drawing or line canvas usage increased in v2, this would multiply the allocation impact - -## Memory Allocation Types - -The allocations fall into several categories: - -1. **String Arrays**: `string[]` from `.ToArray()` -2. **String Lists**: `List` from `.ToList()` -3. **LINQ Enumerable Objects**: Intermediate enumerables in LINQ chains -4. **Dictionary/Collection Allocations**: Less critical but still present - -## GC Impact - -With Gen0 collections potentially happening multiple times per second due to these allocations: - -1. **Pause times**: GC pauses affect UI responsiveness -2. **CPU overhead**: GC work consumes CPU that could render content -3. **Memory pressure**: Constant allocation/collection cycle -4. **Cache pollution**: Reduces cache effectiveness - -## Recommended Solutions (High-Level) - -### For TextFormatter - -1. **Use ArrayPool**: Rent arrays from pool instead of allocating -2. **Use Span**: Work with spans instead of materializing arrays -3. **Cache grapheme arrays**: If text doesn't change, cache the split -4. **Lazy evaluation**: Only materialize when truly needed - -### For LineCanvas - -1. **Apply GetCellMap pattern to GetMap**: Reuse buffer list, use spans -2. **Pool IntersectionDefinition arrays**: Similar to GetCellMap optimization -3. **Consider pixel-level caching**: Cache intersection results for static lines - -### For GraphemeHelper - -1. **Add GetGraphemesAsSpan**: Return `ReadOnlySpan` variant where possible -2. **Add TryGetGraphemeCount**: Count without allocation for validation -3. **Consider string pooling**: Pool common grapheme strings - -## Measurement Recommendations - -To quantify the impact: - -1. **Add BenchmarkDotNet tests**: Measure allocations for typical scenarios -2. **Profile with dotnet-trace**: Capture allocation profiles during Progress demo -3. **Memory profiler**: Use Visual Studio or JetBrains dotMemory - -## Severity by Scenario - -| Scenario | Severity | Reason | -|----------|----------|--------| -| Static UI (no updates) | LOW | Allocations only on initial render | -| Progress bars / animations | **CRITICAL** | Continuous allocations 10-60 Hz | -| Text-heavy UI | **HIGH** | Many text elements = many allocations | -| Border-heavy UI | **HIGH** | Per-pixel allocations in LineCanvas | -| Simple forms | MEDIUM | Periodic allocations on interaction | - -## Conclusion - -The heap allocation issue is **real and significant**, particularly for: - -1. **Any time-based updates** (progress bars, clocks, animations) -2. **Border/line-heavy UIs** due to LineCanvas per-pixel allocations -3. **Text-heavy interfaces** with frequent redraws - -The good news is that the patterns for fixing this are well-established: -- ArrayPool usage -- Span adoption -- Buffer reuse (as demonstrated in GetCellMap) - -The LineCanvas.GetMap() issue is particularly straightforward to fix by applying the same pattern already used in GetCellMap(). - -## Files Requiring Changes - -Priority order: - -1. **Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs** (GetMap method) - CRITICAL -2. **Terminal.Gui/Text/TextFormatter.cs** (Draw method) - CRITICAL -3. **Terminal.Gui/Text/TextFormatter.cs** (other allocation points) - HIGH -4. **Terminal.Gui/Drawing/Cell.cs** (validation) - MEDIUM -5. **Terminal.Gui/Drawing/GraphemeHelper.cs** (add span-based APIs) - MEDIUM - -## Next Steps - -Based on this analysis, the recommendation is to: - -1. ✅ **Acknowledge the issue is real and significant** -2. Fix the most critical issue: LineCanvas.GetMap() per-pixel allocations -3. Fix TextFormatter.Draw() allocations -4. Add benchmarks to measure improvement -5. Consider broader architectural changes for grapheme handling - ---- - -**Analysis Date**: 2025-12-03 -**Analyzed By**: GitHub Copilot -**Codebase**: Terminal.Gui (v2_develop branch) diff --git a/OPTIMIZATION_RECOMMENDATIONS.md b/OPTIMIZATION_RECOMMENDATIONS.md deleted file mode 100644 index 69b9e8c08..000000000 --- a/OPTIMIZATION_RECOMMENDATIONS.md +++ /dev/null @@ -1,546 +0,0 @@ -# Heap Allocation Optimization Recommendations - -## Overview - -This document provides actionable recommendations for addressing the intermediate heap allocation issues identified in Terminal.Gui, with specific priorities and implementation guidance. - -## Priority Ranking - -### P0 - Critical (Must Fix) - -These issues cause severe performance degradation in common scenarios. - -#### 1. LineCanvas.GetMap() Per-Pixel Allocations - -**File:** `Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs` -**Lines:** 210-234 -**Impact:** 1,920+ allocations per border redraw (80×24 window) - -**Problem:** -```csharp -for (int y = inArea.Y; y < inArea.Y + inArea.Height; y++) { - for (int x = inArea.X; x < inArea.X + inArea.Width; x++) { - IntersectionDefinition[] intersects = _lines - .Select(l => l.Intersects(x, y)) - .OfType() - .ToArray(); // ❌ ALLOCATES EVERY PIXEL - } -} -``` - -**Solution:** Apply the same pattern already used in `GetCellMap()`: -```csharp -public Dictionary GetMap (Rectangle inArea) -{ - Dictionary map = new (); - List intersectionsBufferList = []; - - for (int y = inArea.Y; y < inArea.Y + inArea.Height; y++) - { - for (int x = inArea.X; x < inArea.X + inArea.Width; x++) - { - intersectionsBufferList.Clear(); - foreach (var line in _lines) - { - if (line.Intersects(x, y) is { } intersect) - { - intersectionsBufferList.Add(intersect); - } - } - ReadOnlySpan intersects = CollectionsMarshal.AsSpan(intersectionsBufferList); - Rune? rune = GetRuneForIntersects(intersects); - - if (rune is { } && _exclusionRegion?.Contains(x, y) is null or false) - { - map.Add(new (x, y), rune.Value); - } - } - } - return map; -} -``` - -**Expected Improvement:** -- From 1,920 allocations → 1 allocation per redraw (99.95% reduction) -- From 4,800 allocations → 1 allocation for 100×30 dialog -- Immediate, measurable performance gain - -**Effort:** Low (pattern already exists in same class) -**Risk:** Very Low (straightforward refactoring) - ---- - -#### 2. TextFormatter.Draw() Grapheme Array Allocation - -**File:** `Terminal.Gui/Text/TextFormatter.cs` -**Lines:** 126, 934 -**Impact:** Every text draw operation (10-60+ times/second for animated content) - -**Problem:** -```csharp -public void Draw(...) { - // ... - for (int line = lineOffset; line < linesFormatted.Count; line++) { - string strings = linesFormatted[line]; - string[] graphemes = GraphemeHelper.GetGraphemes(strings).ToArray(); // ❌ EVERY DRAW - // ... - } -} -``` - -**Solution Options:** - -**Option A: ArrayPool (Immediate Fix)** -```csharp -using System.Buffers; - -public void Draw(...) { - for (int line = lineOffset; line < linesFormatted.Count; line++) { - string strings = linesFormatted[line]; - - // Estimate or calculate grapheme count - int estimatedCount = strings.Length + 10; // Add buffer for safety - string[] graphemes = ArrayPool.Shared.Rent(estimatedCount); - int actualCount = 0; - - try { - foreach (string grapheme in GraphemeHelper.GetGraphemes(strings)) { - if (actualCount >= graphemes.Length) { - // Need larger array (rare) - string[] larger = ArrayPool.Shared.Rent(graphemes.Length * 2); - Array.Copy(graphemes, larger, actualCount); - ArrayPool.Shared.Return(graphemes); - graphemes = larger; - } - graphemes[actualCount++] = grapheme; - } - - // Use graphemes[0..actualCount] - // ... rest of draw logic ... - - } finally { - ArrayPool.Shared.Return(graphemes, clearArray: true); - } - } -} -``` - -**Option B: Caching (Best for Static Text)** -```csharp -private Dictionary _graphemeCache = new(); -private const int MaxCacheSize = 100; - -private string[] GetGraphemesWithCache(string text) { - if (_graphemeCache.TryGetValue(text, out string[]? cached)) { - return cached; - } - - string[] graphemes = GraphemeHelper.GetGraphemes(text).ToArray(); - - if (_graphemeCache.Count >= MaxCacheSize) { - // Simple LRU: clear cache - _graphemeCache.Clear(); - } - - _graphemeCache[text] = graphemes; - return graphemes; -} -``` - -**Expected Improvement:** -- ArrayPool: Zero allocations for all draws -- Caching: Zero allocations for repeated text (buttons, labels) -- 90-100% reduction in TextFormatter allocations - -**Effort:** Medium -**Risk:** Medium (requires careful array size handling) - -**Recommendation:** Start with Option A (ArrayPool) as it's more robust - ---- - -### P1 - High Priority - -These issues have significant impact in specific scenarios. - -#### 3. TextFormatter Helper Methods - -**Files:** `Terminal.Gui/Text/TextFormatter.cs` -**Lines:** 1336, 1407, 1460, 1726, 2191, 2300 - -**Problem:** Multiple helper methods allocate grapheme arrays/lists - -**Affected Methods:** -- `SplitNewLine()` - Line 1336 -- `ClipOrPad()` - Line 1407 -- `WordWrapText()` - Line 1460 -- `ClipAndJustify()` - Line 1726 -- `GetSumMaxCharWidth()` - Line 2191 -- `GetMaxColsForWidth()` - Line 2300 - -**Solution:** -1. Add overloads that accept `Span` for graphemes -2. Use ArrayPool in calling code -3. Pass pooled arrays to helper methods - -**Example:** -```csharp -// New overload -public static string ClipOrPad(ReadOnlySpan graphemes, int width) { - // Work with span, no allocation -} - -// Caller uses ArrayPool -string[] graphemes = ArrayPool.Shared.Rent(estimatedSize); -try { - int count = FillGraphemes(text, graphemes); - result = ClipOrPad(graphemes.AsSpan(0, count), width); -} finally { - ArrayPool.Shared.Return(graphemes); -} -``` - -**Expected Improvement:** 70-90% reduction in text formatting allocations - -**Effort:** High (multiple methods to update) -**Risk:** Medium (API changes, need careful span handling) - ---- - -#### 4. Cell.Grapheme Validation - -**File:** `Terminal.Gui/Drawing/Cell.cs` -**Line:** 30 - -**Problem:** -```csharp -if (GraphemeHelper.GetGraphemes(value).ToArray().Length > 1) -``` - -**Solution:** -```csharp -// Add helper to GraphemeHelper -public static int GetGraphemeCount(string text) { - if (string.IsNullOrEmpty(text)) return 0; - - int count = 0; - TextElementEnumerator enumerator = StringInfo.GetTextElementEnumerator(text); - while (enumerator.MoveNext()) { - count++; - } - return count; -} - -// In Cell.cs -if (GraphemeHelper.GetGraphemeCount(value) > 1) -``` - -**Expected Improvement:** Zero allocations for Cell.Grapheme validation - -**Effort:** Low -**Risk:** Very Low - ---- - -### P2 - Medium Priority - -Performance improvements for less frequent code paths. - -#### 5. GraphemeHelper API Improvements - -**File:** `Terminal.Gui/Drawing/GraphemeHelper.cs` - -**Additions:** -```csharp -/// Counts graphemes without allocation -public static int GetGraphemeCount(string text); - -/// Fills a span with graphemes, returns actual count -public static int GetGraphemes(string text, Span destination); - -/// Gets graphemes with a rented array from pool -public static (string[] array, int count) GetGraphemesPooled(string text); -``` - -**Benefit:** Provides allocation-free alternatives for all callers - -**Effort:** Medium -**Risk:** Low (additive changes, doesn't break existing API) - ---- - -### P3 - Nice to Have - -Optimizations for edge cases or less common scenarios. - -#### 6. GetDrawRegion Optimization - -**File:** `Terminal.Gui/Text/TextFormatter.cs` -**Line:** 934 - -Similar allocation as Draw method. Apply same ArrayPool pattern. - -**Effort:** Low (copy Draw optimization) -**Risk:** Low - ---- - -## Implementation Roadmap - -### Phase 1: Quick Wins (1-2 days) - -1. ✅ Fix LineCanvas.GetMap() per-pixel allocations -2. ✅ Add GraphemeHelper.GetGraphemeCount() -3. ✅ Fix Cell.Grapheme validation -4. ✅ Add basic benchmarks for measuring improvement - -**Expected Result:** -- Eliminate border/line drawing allocations (99%+ reduction) -- Baseline performance metrics established - -### Phase 2: Core Optimization (3-5 days) - -1. ✅ Implement ArrayPool in TextFormatter.Draw() -2. ✅ Add comprehensive unit tests -3. ✅ Update GetDrawRegion() similarly -4. ✅ Run Progress scenario profiling -5. ✅ Validate allocation reduction - -**Expected Result:** -- TextFormatter allocations reduced 90-100% -- All high-frequency code paths optimized - -### Phase 3: Helpers & API (5-7 days) - -1. ✅ Add GraphemeHelper span-based APIs -2. ✅ Update TextFormatter helper methods -3. ✅ Add optional caching for static text -4. ✅ Full test coverage -5. ✅ Update documentation - -**Expected Result:** -- Complete allocation-free text rendering path -- Public APIs available for consumers - -### Phase 4: Validation & Documentation (2-3 days) - -1. ✅ Run full benchmark suite -2. ✅ Profile Progress demo with dotnet-trace -3. ✅ Compare before/after metrics -4. ✅ Update performance documentation -5. ✅ Create migration guide for API changes - -**Expected Result:** -- Quantified performance improvements -- Clear documentation for maintainers - -**Total Time Estimate:** 2-3 weeks for complete implementation - ---- - -## Testing Strategy - -### Unit Tests - -```csharp -[Theory] -[InlineData("Hello")] -[InlineData("Hello\nWorld")] -[InlineData("Emoji: 👨‍👩‍👧‍👦")] -public void Draw_NoAllocations_WithArrayPool(string text) -{ - var formatter = new TextFormatter { Text = text }; - var driver = new FakeDriver(); - - // Get baseline allocations - long before = GC.GetAllocatedBytesForCurrentThread(); - - formatter.Draw(driver, new Rectangle(0, 0, 100, 10), - Attribute.Default, Attribute.Default); - - long after = GC.GetAllocatedBytesForCurrentThread(); - long allocated = after - before; - - // Should be zero or minimal (some overhead acceptable) - Assert.True(allocated < 1000, $"Allocated {allocated} bytes"); -} -``` - -### Benchmarks - -```csharp -[MemoryDiagnoser] -[BenchmarkCategory("TextFormatter")] -public class DrawBenchmark -{ - private TextFormatter _formatter; - private FakeDriver _driver; - - [GlobalSetup] - public void Setup() - { - _formatter = new TextFormatter { - Text = "Progress: 45%", - ConstrainToWidth = 80, - ConstrainToHeight = 1 - }; - _driver = new FakeDriver(); - } - - [Benchmark] - public void DrawProgressText() - { - _formatter.Draw(_driver, - new Rectangle(0, 0, 80, 1), - Attribute.Default, - Attribute.Default); - } -} -``` - -Expected results: -- **Before:** ~500-1000 bytes allocated per draw -- **After:** 0-100 bytes allocated per draw - -### Performance Testing - -```bash -# Run benchmarks -cd Tests/Benchmarks -dotnet run -c Release --filter "*TextFormatter*" - -# Profile with dotnet-trace -dotnet-trace collect --process-id \ - --providers Microsoft-Windows-DotNETRuntime:0x1:5 - -# Analyze in PerfView or VS -``` - -### Integration Testing - -Run Progress scenario for 60 seconds: -- Monitor GC collections -- Track allocation rate -- Measure frame times -- Compare before/after - ---- - -## Breaking Changes - -### None for Phase 1-2 - -All optimizations are internal implementation changes. - -### Possible for Phase 3 - -If adding span-based APIs: -- New overloads (non-breaking) -- Possible deprecation of allocation-heavy methods - -**Mitigation:** Use `[Obsolete]` attributes with migration guidance - ---- - -## Documentation Updates Required - -1. **CONTRIBUTING.md** - - Add section on allocation-aware coding - - ArrayPool usage guidelines - - Span patterns - -2. **Performance.md** (new file) - - Allocation best practices - - Profiling guide - - Benchmark results - -3. **API Documentation** - - Document allocation behavior - - Note pooled array usage - - Warn about concurrent access (if using pooling) - ---- - -## Monitoring & Validation - -### Success Metrics - -| Metric | Baseline | Target | Measurement | -|--------|----------|--------|-------------| -| Allocations/sec (Progress) | 1,000-5,000 | <100 | Profiler | -| Gen0 GC frequency | Every 5-10s | Every 60s+ | GC.CollectionCount() | -| Frame drops (animated UI) | Occasional | Rare | Frame time monitoring | -| Memory usage (sustained) | Growing | Stable | dotnet-counters | - -### Regression Testing - -Add to CI pipeline: -```yaml -- name: Run allocation benchmarks - run: | - cd Tests/Benchmarks - dotnet run -c Release --filter "*Allocation*" --exporters json - # Parse JSON and fail if allocations exceed baseline -``` - ---- - -## Risk Assessment - -### Low Risk -- LineCanvas.GetMap() fix (proven pattern exists) -- Cell validation fix (simple change) -- Adding new helper methods (additive) - -### Medium Risk -- TextFormatter.Draw() with ArrayPool (complex control flow) -- Span-based API additions (need careful API design) - -### Mitigation -- Comprehensive unit tests -- Gradual rollout (behind feature flag if needed) -- Extensive profiling before merge - ---- - -## Alternative Approaches Considered - -### 1. String Interning -**Pros:** Reduces string allocations -**Cons:** Doesn't solve array allocations, memory pressure -**Decision:** Not suitable for dynamic content - -### 2. Custom Grapheme Iterator -**Pros:** Ultimate control, zero allocations -**Cons:** Complex to implement, maintain -**Decision:** ArrayPool is simpler, achieves same goal - -### 3. Code Generation -**Pros:** Compile-time optimization -**Cons:** Overkill for this problem -**Decision:** Runtime optimization sufficient - ---- - -## Conclusion - -The optimization strategy is: - -1. **Clear and Actionable** - Specific files, lines, and solutions -2. **Proven Patterns** - Uses existing patterns (GetCellMap) and standard tools (ArrayPool) -3. **Measurable** - Clear metrics and testing strategy -4. **Low Risk** - Gradual implementation with extensive testing -5. **High Impact** - 90-99% allocation reduction in critical paths - -**Recommended First Step:** Fix LineCanvas.GetMap() as proof of concept (4-8 hours of work, massive impact) - ---- - -## Questions or Feedback? - -For implementation questions, consult: -- HEAP_ALLOCATION_ANALYSIS.md - Detailed problem analysis -- ALLOCATION_CALL_FLOW.md - Call flow and measurement details -- This document - Implementation roadmap - -**Next Action:** Review with team and prioritize Phase 1 implementation.