Lazily load non-critical contribution modules#316155
Draft
dmitrivMS wants to merge 2 commits into
Draft
Conversation
Move ten contribution modules off the eager workbench bundle parse path: - chat/browser/chat.view.contribution.ts (AfterRestored) - mcp/browser/mcp.view.contribution.ts (AfterRestored) - chat/browser/promptSyntax/promptCodingAgentActionContribution.ts (editor contrib, AfterFirstRender) - chat/browser/promptSyntax/promptToolsCodeLensProvider.ts (editor feature / code lens) - chat/browser/planReviewFeedback/planReviewFeedbackEditorContribution.ts (editor contrib, Eventually) - services/policies/browser/accountPolicyGate.contribution.ts (AfterRestored) - meteredConnection/browser/meteredConnection.contribution.ts (AfterRestored + 1 action) - welcomeOnboarding/browser/welcomeOnboarding.contribution.ts (Delayed IOnboardingService + 1 action) - surveys/browser/nps.contribution.ts (Restored, en-only) - surveys/browser/languageSurveys.contribution.ts (Restored, en-only) None have early consumers: they register views, code lenses, editor contributions, or workbench contributions that only matter once an editor is rendered or the user takes action. A small loader at browser/workbench.contribution.lazy.ts registered at WorkbenchPhase.Eventually dynamically imports them after restore.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces cold-start workbench bundle parsing by removing several non-critical contribution modules from the eager import graph and deferring them via a new lazy-loader contribution that runs at WorkbenchPhase.Eventually.
Changes:
- Remove eager imports for several view/survey/onboarding/policy/metered-connection contributions from
workbench.common.main.ts. - Stop eagerly importing certain chat editor-related contributions from
chat.contribution.ts. - Add
workbench.contribution.lazy.ts, registering a singleEventuallyworkbench contribution that dynamic-import()s the deferred modules.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/workbench.common.main.ts | Drops eager imports for select contributions and adds the new lazy-loader entrypoint import. |
| src/vs/workbench/contrib/chat/browser/chat.contribution.ts | Removes eager imports for prompt/editor-related chat contributions so they can be deferred. |
| src/vs/workbench/browser/workbench.contribution.lazy.ts | New Eventually workbench contribution that dynamic-imports the deferred contribution modules. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 1
Comment on lines
+19
to
+23
| load(() => import('../contrib/chat/browser/chat.view.contribution.js')); | ||
| load(() => import('../contrib/mcp/browser/mcp.view.contribution.js')); | ||
| load(() => import('../contrib/chat/browser/promptSyntax/promptCodingAgentActionContribution.js')); | ||
| load(() => import('../contrib/chat/browser/promptSyntax/promptToolsCodeLensProvider.js')); | ||
| load(() => import('../contrib/chat/browser/planReviewFeedback/planReviewFeedbackEditorContribution.js')); |
Contributor
Author
There was a problem hiding this comment.
Good catch — confirmed. EditorFeaturesInstantiator (editorFeatures.ts:13–53) snapshots getEditorFeatures() exactly once (gated by _instantiated) at BlockRestore / first editor creation, and CodeEditorWidget reads EditorExtensionsRegistry at construction time. Both are non-retroactive, so deferring these three modules to Eventually could leave them unattached.
Reverted in commit a833565f778 — promptCodingAgentActionContribution.ts, promptToolsCodeLensProvider.ts, and planReviewFeedbackEditorContribution.ts are back on the eager path. I also expanded the lazy set with eight more registerWorkbenchContribution2-only modules in the same commit, all of which are retroactive per contributions.ts:170–180.
dmitrivMS
changed the title
Address Copilot review feedback: revert lazy-loading of three modules that use `registerEditorFeature` / `registerEditorContribution`. Those registries are non-retroactive — `EditorFeaturesInstantiator` (registered at `BlockRestore`) snapshots `getEditorFeatures()` once on first editor creation, and `CodeEditorWidget` snapshots `EditorExtensionsRegistry` at editor construction time. Reverted to eager: - chat/browser/promptSyntax/promptCodingAgentActionContribution.ts - chat/browser/promptSyntax/promptToolsCodeLensProvider.ts - chat/browser/planReviewFeedback/planReviewFeedbackEditorContribution.ts Newly deferred (workbench-contribution-only, all retroactive via `registerWorkbenchContribution2`): - bracketPairColorizer2Telemetry.contribution.ts (Restored) - scrollLocking.contribution.ts (Eventually) - dropOrPasteInto.contribution.ts (Eventually x2) - opener.contribution.ts (Eventually) - relauncher.contribution.ts (Restored x2) - update.contribution.ts (Restored x5 + Eventually) - sash.contribution.ts (AfterRestored) - languageDetection.contribution.ts (Restored)
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
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.
What
Defers fifteen contribution modules off the eager workbench bundle parse path. A new loader at
src/vs/workbench/browser/workbench.contribution.lazy.tsregisters a single workbench contribution atWorkbenchPhase.Eventuallythat dynamic-import()s the deferred modules after the workbench has fully restored.chat/browser/chat.view.contribution.tsmcp/browser/mcp.view.contribution.tsservices/policies/browser/accountPolicyGate.contribution.tsmeteredConnection/browser/meteredConnection.contribution.tswelcomeOnboarding/browser/welcomeOnboarding.contribution.tsIOnboardingServiceDelayed + 1 actionsurveys/browser/nps.contribution.tssurveys/browser/languageSurveys.contribution.tsbracketPairColorizer2Telemetry/browser/bracketPairColorizer2Telemetry.contribution.tsscrollLocking/browser/scrollLocking.contribution.tsdropOrPasteInto/browser/dropOrPasteInto.contribution.tsopener/browser/opener.contribution.tsrelauncher/browser/relauncher.contribution.tsupdate/browser/update.contribution.tssash/browser/sash.contribution.tslanguageDetection/browser/languageDetection.contribution.tsWhy this is safe
The workbench contributions registry instantiates a contribution immediately if it is registered after its target phase has already fired (
contributions.ts:170–180). All fifteen deferred modules register only viaregisterWorkbenchContribution2, which is retroactive.Modules that use non-retroactive mechanisms were deliberately not included (or reverted, see review thread):
registerEditorFeature(...)— snapshot taken once byEditorFeaturesInstantiatoratBlockRestoreon first editor creation; not re-read.registerEditorContribution(...)—CodeEditorWidgetsnapshotsEditorExtensionsRegistryat editor construction time.registerEditorPane/registerEditorSerializer/registerExtensionPoint— consumed during workspace/editor restore or extension activation, which happen beforeEventually.That's why
chatContext.contribution.ts,imageCarousel.contribution.ts,chatSessions.contribution.ts,agentSessions.contribution.ts,chatManagement.contribution.ts,aiCustomizationManagement.contribution.ts, the three prompt/plan-review editor contributions, and modules whose services are consumed bymainThread*bridges (share,editTelemetry,accessibilitySignals) are kept eager.Impact
Transitive-closure analysis on the previous (18-module) variant of this set showed ~200 KB of TS source uniquely leaves the eager parse graph — most transitive imports are shared with other eager paths and cost nothing extra to dynamic-import. The current (15-module) set is similar in net size: the three reverted modules together accounted for ~30 KB of direct source plus minor transitive surface.
Estimated cold-start savings:
First in a planned series of lazy-loading changes (tracked locally in
perf-changes.md).Trade-offs
Eventuallyinstead of duringRestored. All gated and rare.SettingsChangeRelauncher/WorkspaceChangeExtHostRelauncher: settings changes made in the first ~2.5 s of session won't be observed. Users do not normally change settings that early.No functionality removed.
Validation
npm run compile-check-ts-native— cleannpm run valid-layers-check— clean