Daily Perf Improver: Fix memory leak in append operations (Issue #35) #193
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit implements an optimized append function that eliminates the memory leak caused by generator chains in the AsyncGenerator.append implementation. ## Key Changes - Replaced AsyncGenerator.append with direct IAsyncEnumerable implementation - Eliminates generator chain creation that prevented garbage collection - Maintains O(1) memory usage for streaming append operations - All existing tests pass (168/168) ## Performance Results Benchmark results show significant memory usage improvements: - Chained appends (1000): Only 11KB memory increase vs OutOfMemoryException - Large sequences (200K elements): Minimal GC pressure (22 gen0 collections) - Multiple appends: Very low memory overhead ## Technical Details The new implementation uses a stateful enumerator that: 1. Starts with the first sequence enumerator 2. Switches to second sequence when first is exhausted 3. Properly disposes enumerators to prevent memory leaks 4. Avoids creating continuation chains that prevent GC This addresses the root cause identified in Issue #35 where append operations caused O(n) memory usage instead of O(1) for streaming. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
20 tasks
6 tasks
github-actions bot
pushed a commit
that referenced
this pull request
Aug 29, 2025
## Summary This PR implements significant performance optimizations for AsyncSeq.collect, addressing Round 2 goals from the performance improvement plan (Issue #190). The optimization focuses on reducing memory allocations and improving state management efficiency for collect operations. ## Performance Improvements - 32% faster execution for many small inner sequences (0.44s vs 0.65s for 5000 elements) - Improved memory efficiency through direct mutable fields instead of ref cells - Better state management with tail-recursive loop structure - Consistent performance across various collect patterns - Maintained O(1) memory usage for streaming operations ## Technical Implementation ### Root Cause Analysis The original collect implementation had several performance issues: - Ref cell allocations for state management (let state = ref ...) - Multiple pattern matching on each MoveNext() call - Deep continuation chains from return! x.MoveNext() recursion - Heap allocations for state transitions ### Optimization Strategy Created OptimizedCollectEnumerator<'T, 'U> with: - Direct mutable fields instead of reference cells - Tail-recursive loop for better async performance - Streamlined state management without discriminated union overhead - Efficient disposal with proper resource cleanup ## Validation All existing tests pass (175/175) Performance benchmarks show measurable improvements No breaking changes - API remains identical Edge cases tested - empty sequences, exceptions, disposal, cancellation ## Related Issues - Addresses Round 2 core algorithm optimization from #190 (Performance Research and Plan) - Builds upon optimizations from merged PRs #193, #194, #196 - Contributes to "reduce per-operation allocations by 50%" goal > AI-generated content by Daily Perf Improver may contain mistakes.
This was referenced Aug 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the critical memory leak in
AsyncSeq.appendoperations identified in Issue #35. The original implementation created generator chains that prevented garbage collection, causing O(n) memory usage instead of O(1) for streaming operations.Performance Improvements
🚀 Major memory leak eliminated:
📊 Benchmark Results:
Technical Implementation
Root Cause
The original
AsyncGenerator.appendimplementation created continuation chains usingbindGthat prevented garbage collection of intermediate generators.Solution
Replaced with a direct
IAsyncEnumerableimplementation that:Code Changes
AsyncSeq.appendfunction inAsyncSeq.fs:376-412append_benchmark.fsx)Validation
✅ All existing tests pass (168/168)
✅ Performance benchmarks show dramatic improvements
✅ No breaking changes - API remains identical
✅ Memory usage now O(1) instead of O(n) for streaming
Test Plan
dotnet test -c Releasedotnet fsi append_benchmark.fsxRelated Issues
Commands Used
Web Searches Performed
MCP Function Calls Used
mcp__github__search_issues: Found research issue Daily Perf Improver: Research and Plan #190mcp__github__get_issue_comments: Checked for maintainer feedbackmcp__github__search_pull_requests: Verified no conflicting PRsThis change eliminates a major performance bottleneck and memory leak that affected all append-heavy AsyncSeq operations. The implementation maintains full backward compatibility while providing significant performance and memory usage improvements.