|
| 1 | +# Benchmarking Plan for RemoveObjectFromList Performance (Issue #221) |
| 2 | + |
| 3 | +## Overview |
| 4 | +This plan addresses the performance issue reported in [Issue #221](https://github.com/NetOfficeFw/NetOffice/issues/221) regarding `Core.RemoveObjectFromList` performance bottleneck. |
| 5 | + |
| 6 | +## Problem Statement |
| 7 | +The current implementation uses `List<ICOMObject>` for `_globalObjectList`, resulting in O(n²) complexity when disposing parent objects with multiple children. Each `Remove()` operation is O(n), and disposing N children results in cumulative O(n²) time complexity. |
| 8 | + |
| 9 | +## Current State Analysis |
| 10 | + |
| 11 | +### Code Location |
| 12 | +- File: `Source/NetOffice/Core.cs` |
| 13 | +- Field: `_globalObjectList` (line 96) |
| 14 | +- Method: `RemoveObjectFromList()` (lines 1346-1367) |
| 15 | + |
| 16 | +### Current Implementation |
| 17 | +```csharp |
| 18 | +private List<ICOMObject> _globalObjectList = new List<ICOMObject>(); |
| 19 | + |
| 20 | +internal void RemoveObjectFromList(ICOMObject proxy, IEnumerable<ICOMObject> ownerPath) |
| 21 | +{ |
| 22 | + lock (_globalObjectList) |
| 23 | + { |
| 24 | + removed = _globalObjectList.Remove(proxy); // O(n) operation |
| 25 | + } |
| 26 | +} |
| 27 | +``` |
| 28 | + |
| 29 | +### Testing Infrastructure |
| 30 | +- **Framework:** NUnit |
| 31 | +- **Location:** `/Users/izso/Developer/NetOffice/Source/NetOffice.Tests/` |
| 32 | +- **Current State:** No benchmarking infrastructure exists |
| 33 | +- **Need:** Create new benchmark project with BenchmarkDotNet |
| 34 | + |
| 35 | +## Benchmark Plan |
| 36 | + |
| 37 | +### 1. Infrastructure Setup |
| 38 | + |
| 39 | +**Create New Benchmark Project:** |
| 40 | +- Project name: `NetOffice.Benchmarks` |
| 41 | +- Location: `/Users/izso/Developer/NetOffice/Benchmarks/` or `/Users/izso/Developer/NetOffice/Tests/Benchmarks/` |
| 42 | +- Dependencies: |
| 43 | + - BenchmarkDotNet (latest stable) |
| 44 | + - NetOffice.Core (project reference) |
| 45 | +- Target frameworks: Match main project (.NET Framework, .NET Core, etc.) |
| 46 | + |
| 47 | +### 2. Benchmark Scenarios |
| 48 | + |
| 49 | +Test with varying object counts: **10, 100, 1,000, 10,000 objects** |
| 50 | + |
| 51 | +#### Scenario A: Sequential Removal |
| 52 | +**Purpose:** Simulate disposing a parent with N children (worst-case for List) |
| 53 | + |
| 54 | +**Test:** |
| 55 | +```csharp |
| 56 | +[Benchmark] |
| 57 | +public void SequentialRemoval_List() |
| 58 | +{ |
| 59 | + // Add N objects to List |
| 60 | + // Remove all objects one by one |
| 61 | + // Measures O(n²) complexity |
| 62 | +} |
| 63 | +``` |
| 64 | + |
| 65 | +**Expected Outcome:** Performance degrades quadratically with List, linearly with HashSet |
| 66 | + |
| 67 | +#### Scenario B: Bulk Disposal |
| 68 | +**Purpose:** Test `DisposeAllCOMProxies()` behavior |
| 69 | + |
| 70 | +**Test:** |
| 71 | +```csharp |
| 72 | +[Benchmark] |
| 73 | +public void BulkDisposal_List() |
| 74 | +{ |
| 75 | + // Add N objects |
| 76 | + // Remove from end in while loop (current pattern) |
| 77 | +} |
| 78 | +``` |
| 79 | + |
| 80 | +**Expected Outcome:** Better than sequential for List, similar for both |
| 81 | + |
| 82 | +#### Scenario C: Mixed Operations |
| 83 | +**Purpose:** Real-world usage with add/remove interleaved |
| 84 | + |
| 85 | +**Test:** |
| 86 | +```csharp |
| 87 | +[Benchmark] |
| 88 | +public void MixedOperations() |
| 89 | +{ |
| 90 | + // 70% adds, 30% removes |
| 91 | + // Simulate realistic COM object lifecycle |
| 92 | +} |
| 93 | +``` |
| 94 | + |
| 95 | +**Expected Outcome:** Tests practical performance under lock contention |
| 96 | + |
| 97 | +#### Scenario D: Memory Allocation |
| 98 | +**Purpose:** Compare memory overhead and GC pressure |
| 99 | + |
| 100 | +**Test:** |
| 101 | +```csharp |
| 102 | +[MemoryDiagnoser] |
| 103 | +[Benchmark] |
| 104 | +public void MemoryFootprint() |
| 105 | +{ |
| 106 | + // Track allocations for List vs HashSet |
| 107 | + // Measure GC collections |
| 108 | +} |
| 109 | +``` |
| 110 | + |
| 111 | +**Expected Outcome:** HashSet has higher base memory but better scaling |
| 112 | + |
| 113 | +### 3. Implementation Variants to Compare |
| 114 | + |
| 115 | +#### Variant 1: Current Implementation (Baseline) |
| 116 | +```csharp |
| 117 | +private List<ICOMObject> _globalObjectList = new List<ICOMObject>(); |
| 118 | +``` |
| 119 | +- Time: O(n) per removal |
| 120 | +- Memory: O(n) |
| 121 | +- Thread-safety: Manual locking required |
| 122 | + |
| 123 | +#### Variant 2: HashSet (Proposed) |
| 124 | +```csharp |
| 125 | +private HashSet<ICOMObject> _globalObjectList = new HashSet<ICOMObject>(); |
| 126 | +``` |
| 127 | +- Time: O(1) per removal |
| 128 | +- Memory: O(n) with higher constant factor |
| 129 | +- Thread-safety: Manual locking required |
| 130 | +- **Caveat:** Requires proper `GetHashCode()` and `Equals()` implementation on ICOMObject |
| 131 | + |
| 132 | +#### Variant 3: Dictionary (Alternative) |
| 133 | +```csharp |
| 134 | +private Dictionary<IntPtr, ICOMObject> _globalObjectList = new Dictionary<IntPtr, ICOMObject>(); |
| 135 | +``` |
| 136 | +- Time: O(1) per removal by key |
| 137 | +- Memory: O(n) |
| 138 | +- Thread-safety: Manual locking required |
| 139 | +- **Benefit:** Can key by COM pointer for guaranteed uniqueness |
| 140 | + |
| 141 | +#### Variant 4: ConcurrentDictionary (Thread-safe) |
| 142 | +```csharp |
| 143 | +private ConcurrentDictionary<IntPtr, ICOMObject> _globalObjectList = new ConcurrentDictionary<IntPtr, ICOMObject>(); |
| 144 | +``` |
| 145 | +- Time: O(1) per removal (average) |
| 146 | +- Memory: O(n) with higher overhead |
| 147 | +- Thread-safety: Built-in lock-free operations |
| 148 | +- **Benefit:** Reduces lock contention |
| 149 | + |
| 150 | +### 4. Metrics to Capture |
| 151 | + |
| 152 | +**Performance Metrics:** |
| 153 | +- Mean execution time |
| 154 | +- Median execution time |
| 155 | +- 99th percentile (p99) |
| 156 | +- Standard deviation |
| 157 | +- Operations per second |
| 158 | + |
| 159 | +**Memory Metrics:** |
| 160 | +- Total allocated bytes |
| 161 | +- Gen 0/1/2 GC collections |
| 162 | +- Memory footprint per object count |
| 163 | + |
| 164 | +**Scalability Metrics:** |
| 165 | +- Time complexity verification (plot time vs N) |
| 166 | +- Memory scaling (plot memory vs N) |
| 167 | + |
| 168 | +**Configuration:** |
| 169 | +```csharp |
| 170 | +[MemoryDiagnoser] |
| 171 | +[SimpleJob(RuntimeMoniker.Net48)] |
| 172 | +[SimpleJob(RuntimeMoniker.Net60)] |
| 173 | +[SimpleJob(RuntimeMoniker.Net80)] |
| 174 | +public class RemoveObjectBenchmark |
| 175 | +{ |
| 176 | + [Params(10, 100, 1000, 10000)] |
| 177 | + public int ObjectCount; |
| 178 | +} |
| 179 | +``` |
| 180 | + |
| 181 | +### 5. Implementation Steps |
| 182 | + |
| 183 | +1. ✅ Research existing test/benchmark infrastructure |
| 184 | +2. ⏳ Design benchmark scenarios (this document) |
| 185 | +3. ⏳ Create `NetOffice.Benchmarks` project |
| 186 | +4. ⏳ Implement benchmark classes for all variants |
| 187 | +5. ⏳ Create mock ICOMObject implementation for testing |
| 188 | +6. ⏳ Run benchmarks and collect data |
| 189 | +7. ⏳ Generate comparison charts/tables |
| 190 | +8. ⏳ Document findings and recommendations |
| 191 | +9. ⏳ Update Issue #221 with results |
| 192 | + |
| 193 | +### 6. Expected Deliverables |
| 194 | + |
| 195 | +**Code:** |
| 196 | +- `NetOffice.Benchmarks` project |
| 197 | +- `RemoveObjectListBenchmark.cs` - Main benchmark class |
| 198 | +- `CoreVariants.cs` - Different implementation approaches |
| 199 | +- `MockCOMObject.cs` - Test harness |
| 200 | + |
| 201 | +**Documentation:** |
| 202 | +- `BenchmarkResults.md` - Raw benchmark output |
| 203 | +- `PerformanceAnalysis.md` - Analysis and recommendations |
| 204 | +- Charts/graphs showing performance comparison |
| 205 | +- Memory profiling results |
| 206 | + |
| 207 | +**GitHub:** |
| 208 | +- Comment on Issue #221 with findings |
| 209 | +- Recommendation for implementation change |
| 210 | +- Pull request (if proceeding with fix) |
| 211 | + |
| 212 | +### 7. Success Criteria |
| 213 | + |
| 214 | +**Validation Goals:** |
| 215 | +1. Confirm O(n²) vs O(n) complexity difference |
| 216 | +2. Demonstrate measurable performance improvement (target: >10x for N=10,000) |
| 217 | +3. Verify no regression in memory usage (acceptable: <2x increase) |
| 218 | +4. Ensure thread-safety is maintained |
| 219 | +5. Validate that `ICOMObject.Equals()` and `GetHashCode()` work correctly |
| 220 | + |
| 221 | +**Decision Points:** |
| 222 | +- If HashSet shows >10x improvement with <2x memory increase → Recommend implementation |
| 223 | +- If thread contention is significant → Consider ConcurrentDictionary |
| 224 | +- If ICOMObject equality is unreliable → Use Dictionary with IntPtr key |
| 225 | + |
| 226 | +### 8. Risks and Considerations |
| 227 | + |
| 228 | +**Risks:** |
| 229 | +1. **Equality Implementation:** ICOMObject must implement proper equality for HashSet |
| 230 | +2. **Breaking Changes:** Behavior change if list order was relied upon |
| 231 | +3. **Memory Overhead:** HashSet has higher memory overhead than List |
| 232 | +4. **COM Interop Complexity:** Performance may vary with real COM objects |
| 233 | + |
| 234 | +**Mitigations:** |
| 235 | +1. Verify ICOMObject.Equals() implementation before testing |
| 236 | +2. Check codebase for any order-dependent code |
| 237 | +3. Measure memory increase and document trade-offs |
| 238 | +4. Consider hybrid approach if needed |
| 239 | + |
| 240 | +### 9. Future Enhancements |
| 241 | + |
| 242 | +If benchmarking infrastructure proves valuable: |
| 243 | +- Benchmark other performance-critical paths |
| 244 | +- Add continuous performance monitoring to CI/CD |
| 245 | +- Create performance regression tests |
| 246 | +- Profile memory leaks in COM object lifecycle |
| 247 | + |
| 248 | +## References |
| 249 | + |
| 250 | +- Issue: https://github.com/NetOfficeFw/NetOffice/issues/221 |
| 251 | +- Code: `Source/NetOffice/Core.cs:96, 1346-1367` |
| 252 | +- BenchmarkDotNet: https://benchmarkdotnet.org/ |
| 253 | +- Related: Performance analysis of .NET collections |
| 254 | + |
| 255 | +## Notes |
| 256 | + |
| 257 | +This plan assumes: |
| 258 | +- Modern .NET SDK is available for BenchmarkDotNet |
| 259 | +- ICOMObject has stable equality/hash implementation |
| 260 | +- Benchmark runs can be performed on development machine |
| 261 | +- Results will be validated across different .NET runtime versions |
0 commit comments