Skip to content

Commit a02ca5f

Browse files
CopilotT-Gro
andcommitted
Document O(n²) root cause - iteration not append, further optimization needed
Updated PERFORMANCE_RESULTS.md with 10K findings: - CachedDList fixes append: O(n)→O(1) ✅ - Issue persists: AllEntitiesByLogicalMangledName iterates all entities O(n) per file - Total complexity: O(n²) from iteration, not append - 5K files: no regression (17s) - 10K files: >22min (quadratic confirmed) Recommendation: Cache AllEntitiesByLogicalMangledName across merges (future work) Co-authored-by: T-Gro <[email protected]>
1 parent 7d58987 commit a02ca5f

File tree

1 file changed

+46
-4
lines changed

1 file changed

+46
-4
lines changed

investigation/dlist_performance/PERFORMANCE_RESULTS.md

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,55 @@ From QueueListBenchmarks.fs (5000 sequential appends):
8080
- Expected benefit at 10K+ files where O(n²) becomes problematic
8181
- Original issue (fsharp-10k) should see dramatic improvement
8282

83+
## 10,000 Files Test Results
84+
85+
### ⚠️ O(n²) Issue Persists
86+
87+
| Test | Time | Memory | Status |
88+
|------|------|--------|--------|
89+
| **CachedDList** | >22 minutes | ~14 GB | Running |
90+
| **Original Issue** | >10 minutes (killed) | 15GB+ | Matches reported |
91+
92+
### Root Cause: Iteration, Not Append
93+
94+
The O(n²) complexity in `CombineModuleOrNamespaceTypes` comes from **entity iteration**, not append:
95+
96+
```fsharp
97+
// Called once per file merge:
98+
let entities1ByName = mty1.AllEntitiesByLogicalMangledName // O(n) - iterates ALL entities
99+
let entities2ByName = mty2.AllEntitiesByLogicalMangledName // O(m) - iterates new entities
100+
// Conflict checking also iterates
101+
// Total: O(n) per file × n files = O(n²)
102+
```
103+
104+
**What CachedDList fixes:**
105+
- ✅ Append: O(n) → O(1) (4.1x faster)
106+
- ✅ No regression at 5K files
107+
108+
**What remains unfixed:**
109+
- ⚠️ `AllEntitiesByLogicalMangledName` rebuilds map from ALL entities
110+
- ⚠️ Called once per file → O(n²) total
111+
112+
### Recommendation
113+
114+
**Additional optimizations needed:**
115+
1. Cache `AllEntitiesByLogicalMangledName` across merges
116+
2. Incremental map updates instead of full rebuilds
117+
3. Or restructure to avoid repeated iteration of all entities
118+
119+
**CachedDList is still valuable:**
120+
- Improves typical projects (<5K files)
121+
- Necessary architectural improvement
122+
- Foundation for future optimizations
123+
83124
## Next Steps
84125

85126
1.**Validation Complete**: CachedDList migration successful
86-
2. 🧪 **Test with 10,000 files**: Validate improvement on original issue
87-
3. 📝 **Document**: Update PR with performance results
88-
4. 🔍 **Code Review**: Request review of changes
89-
5. 🚀 **Merge**: Ready for integration
127+
2.**Test with 10,000 files**: O(n²) confirmed, root cause identified
128+
3. 📝 **Document**: Findings documented
129+
4. 🔧 **Further optimization**: Cache AllEntitiesByLogicalMangledName (future work)
130+
5. 🔍 **Code Review**: Request review of CachedDList changes
131+
6. 🚀 **Merge**: CachedDList ready (no regressions, improves append)
90132

91133
## Files Generated
92134
- `build_output.txt` - CachedDList compiler build output

0 commit comments

Comments
 (0)