-
Notifications
You must be signed in to change notification settings - Fork 37.3k
chat: fix performance slowdowns when closing/reloading the window #287186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- _onDidChangeToolsScheduler.isScheduled is checked to avoid thrashing setTimeout when disposing many tools - WorkspaceExtensionsManagementService was listening to file change events using a debounce. Debounces have overhead because a new timer is scheduled on every single call. For large amount of file changes (during EH shutdown when schemas for tools are deregistered) this caused a notable slowdown. `throttle` should be functionally equivalent. - avoid triggering input updates (w/ downstream editor effects) each time the input gets parsed, which happened every time tool is called - big hammer -- don't bother deregistering MCP tools each time Results: - `216ms` to shut down EH before making these changes - `87ms` in the first three bullets - `54ms` after skipping MCP tool deregistering. (Basically all the overhead there was unregistering the JSON schema for tool inputs.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aims to fix performance slowdowns when closing or reloading the VS Code window, specifically targeting chat and MCP tool-related overhead during Extension Host shutdown. The changes implement several optimizations to reduce shutdown time from 216ms to 54ms.
Changes:
- Replaced Event.debounce with Event.throttle for file change handling in workspace extensions management
- Added shutdown optimization to skip MCP tool cleanup during window close
- Implemented throttle event utility with comprehensive tests
- Added equals comparison for parsed chat requests to avoid redundant updates
- Optimized tool registration scheduler to prevent redundant scheduling
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/base/common/event.ts | Adds new Event.throttle utility for rate-limiting event firing |
| src/vs/base/test/common/event.test.ts | Adds comprehensive test coverage for throttle function |
| src/vs/editor/common/core/ranges/offsetRange.ts | Adds static equals method for comparing offset ranges |
| src/vs/workbench/contrib/chat/common/requestParser/chatParserTypes.ts | Adds equals method for comparing parsed chat requests |
| src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts | Uses equals comparison to avoid firing redundant input change events |
| src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts | Checks scheduler state before scheduling to avoid redundant setTimeout calls |
| src/vs/workbench/services/extensionManagement/common/extensionManagementService.ts | Switches from debounce to throttle for file change handling |
| src/vs/workbench/contrib/mcp/common/mcpLanguageModelToolContribution.ts | Attempts to skip tool cleanup during shutdown for performance |
src/vs/workbench/contrib/mcp/common/mcpLanguageModelToolContribution.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/services/extensionManagement/common/extensionManagementService.ts
Show resolved
Hide resolved
…bution.ts Co-authored-by: Copilot <[email protected]>
throttleshould be functionally equivalent, cc @sandy081Results:
216msto shut down EH before making these changes87msin the first three bullets54msafter skipping MCP tool deregistering. (Basically all the overhead there was unregistering the JSON schema for tool inputs.)Closes #287174