-
Notifications
You must be signed in to change notification settings - Fork 38.2k
Description
GitHub Issue: Review AsyncIterableObject usage: potential memory leaks and migration to AsyncIterableProducer
Title: Review AsyncIterableObject usage: potential memory leaks and migration to AsyncIterableProducer
Problem
AsyncIterableObject has a potential memory leak issue when iterables are only consumed once. The class stores all emitted values in an internal _results array to support multiple iterations, but this means values are never garbage collected in long-running iterations even when the iterable is only consumed once.
AsyncIterableProducer was introduced as an alternative that doesn't store values, making it more memory-efficient for single-consumption scenarios. However, this means that a second iteration will miss initial values.
Analysis
We found 112 occurrences of AsyncIterableObject across the codebase that should be reviewed to determine if they can be migrated to AsyncIterableProducer for better memory efficiency.
Action Items
Please review the usages you've contributed to and determine if they can be migrated to AsyncIterableProducer:
Contributors to Review
- @alexdima (31 occurrences) - Core async.ts implementation and editor hover functionality
- @jrieken (18 occurrences) - AsyncIterableSource, tests, chat and language model features
- @aiday-mar (15 occurrences) - Editor hover functionality (inlay hints, color picker, markdown hover)
- @amunger (19 occurrences total) - Notebook-related functionality and tests
- @hediet (8 occurrences) - Core async functionality and inline completions
- @Tyriar (5 occurrences) - Terminal shell integration
- @sandy081 (4 occurrences) - Extension services
- @joaomoreno (2 occurrences) - Workspace functionality
- @connor4312 (2 occurrences) - MCP (Model Context Protocol) server
- @aeschli (2 occurrences) - Chat code block operations
- @DonJayamanne (1 occurrence) - Chat code block operations
- @mjbvz (1 occurrence) - Editor hover functionality
Review Criteria
For each usage, consider:
- Single vs Multiple Consumption: Is the AsyncIterable consumed only once or multiple times?
- Memory Sensitivity: Are large amounts of data being emitted that should be garbage collected promptly?
- Performance Impact: Would switching to AsyncIterableProducer provide meaningful memory savings?
Migration Guidelines
✅ Good candidates for AsyncIterableProducer:
- Single-use iterables (consumed once then discarded)
- Large data streams
- Real-time data processing
- Event streams
❌ Keep AsyncIterableObject for:
- Iterables that need to be consumed multiple times
- Cases where re-iteration is expected
Files to Review
Core Implementation (31 occurrences)
src/vs/base/common/async.ts
- Lines: 1963, 1975, 1977, 1978, 1983, 1984, 1989, 1990, 1995, 1996, 2005, 2063, 2064, 2071, 2072, 2075, 2076, 2085, 2086, 2087, 2088, 2091, 2092, 2095, 2096, 2108, 2171, 2184, 2188, 2215, 2228, 2267, 2375
Editor Features (38 occurrences)
Hover functionality:
- src/vs/editor/contrib/hover/browser/contentHoverComputer.ts (5 occurrences)
- src/vs/editor/contrib/hover/browser/markdownHoverParticipant.ts (5 occurrences)
- src/vs/editor/contrib/hover/browser/getHover.ts (3 occurrences)
- src/vs/editor/contrib/hover/browser/hoverOperation.ts (3 occurrences)
- src/vs/editor/contrib/hover/browser/hoverTypes.ts (2 occurrences)
Inlay hints:
- src/vs/editor/contrib/inlayHints/browser/inlayHintsHover.ts (7 occurrences)
Other editor features:
- src/vs/editor/contrib/inlineCompletions/browser/model/provideInlineCompletions.ts (3 occurrences)
- src/vs/editor/contrib/colorPicker/browser/hoverColorPicker/hoverColorPickerParticipant.ts (3 occurrences)
Workbench Features (24 occurrences)
Notebook functionality:
- src/vs/workbench/contrib/notebook/test/browser/ (15 occurrences across test files)
- src/vs/workbench/contrib/notebook/common/notebookKernelService.ts (2 occurrences)
- src/vs/workbench/api/browser/mainThreadNotebookKernels.ts (3 occurrences)
Chat functionality:
- src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts (2 occurrences)
- src/vs/workbench/contrib/chat/browser/actions/codeBlockOperations.ts (2 occurrences)
Extension and API Features (19 occurrences)
- src/vs/workbench/services/extensions/browser/extensionService.ts (2 occurrences)
- src/vs/workbench/services/extensions/electron-browser/nativeExtensionService.ts (2 occurrences)
- src/vs/workbench/api/common/extHostTerminalShellIntegration.ts (4 occurrences)
- src/vs/workbench/api/common/extHostLanguageModels.ts (2 occurrences)
- src/vs/workbench/api/common/extHostWorkspace.ts (2 occurrences)
- src/vs/workbench/contrib/mcp/common/mcpServer.ts (2 occurrences)
Next Steps
- Each contributor should review their assigned usages
- Create follow-up PRs for migrations where appropriate
- Update documentation/guidelines for when to use each class
- Consider adding ESLint rules to guide future usage
This review will help improve VS Code's memory efficiency and prevent potential memory leaks in async iteration scenarios.
Labels to add: debt, performance, good first issue, help wanted