Skip to content

Review AsyncIterableObject usage: potential memory leaks and migration to AsyncIterableProducer #256854

@hediet

Description

@hediet

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:

  1. Single vs Multiple Consumption: Is the AsyncIterable consumed only once or multiple times?
  2. Memory Sensitivity: Are large amounts of data being emitted that should be garbage collected promptly?
  3. 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

  1. Each contributor should review their assigned usages
  2. Create follow-up PRs for migrations where appropriate
  3. Update documentation/guidelines for when to use each class
  4. 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

Metadata

Metadata

Labels

debtCode quality issuesgood first issueIssues identified as good for first-time contributorshelp wantedIssues identified as good community contribution opportunitiesperf

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions