Pull Request Overview
- Opened on September 18, 2026
- Status Merged
- Commit count 1 with first commit September 18, 2026
Total Delta
Open Days
Test Delta
How long has this pull request spent in each phase of its lifecycle?
| Fraction of total time | Business days | Phase |
|---|---|---|
| N/A | 1.1 days | Issue creation to work beginning |
|
|
0.0 days | Authoring 1 commit before pull request opened for review |
|
|
0.0 days | Awaiting first review |
|
|
0.6 days | Revising work with 0 commits in response to 4 reviews that left 2 comments |
|
|
0.2 days | Merge to deploy (still awaiting deployment) |
Total time for pull request
0.6 business days
from first commit authored to merge
Still awaiting deployment
menus: Optimize one-shot menu action resolution
Summary
Optimize the menu-construction part of #336496 without adding a product-wide cache or changing tree-find behavior.
- Resolve one-shot menu actions directly instead of constructing and disposing an event-tracking
MenuImpl. - Filter contributions by the current context before sorting, and skip collection of context-key dependencies that only persistent menus need. Apply the same path to submenus.
- Share action construction with persistent menus, preserving action arguments, alternate commands, enablement, toggled state, hidden state, and submenu notifications.
- Remove the redundant second
MenuInfo.refresh()(fixes #336495). - Normalize empty and omitted group names to the same default group in the comparator. This makes sorting consistent with grouping and avoids order differences between filtering before and after sorting.
There is deliberately no cross-call cache or registry-version API: each one-shot request rereads the registry, so in-place descriptor changes and implicit Command Palette exclusions remain current. No per-menu or per-row retained state is added.
Validation
-
npm run gulp compile-client— passed, zero errors. - Hygiene/ESLint on all three changed files — passed.
- Electron
MenuServicesuite — 19 tests passed, including 13 new tests. - Three targeted performance/ordering regressions fail against the original implementation and pass with this change.
- 200 deterministic mixed-group comparisons matched one-shot and persistent-menu output.
Local Windows microbenchmark: 875 registered items, 100 rows, two lookups per row; median of nine alternating runs, with output parity checked and no concurrent build.
| Scenario | Before | After | Speedup |
| --- | ---: | ---: | ---: |
| Row-specific context (16 context values) | 939.3 ms | 74.5 ms | 12.6x |
| All contributions applicable | 1944.1 ms | 1515.1 ms | 1.28x |
These are menu-resolution measurements, not an end-to-end reproduction of the reported 20-second freeze. The tree find widget's full-list refilter remains unchanged, so #336496 should remain open for that separate part.
<!-- ⚠️⚠️ Do Not Delete This! bug_report_template ⚠️⚠️ -->
<!-- Please read our Rules of Conduct: https://opensource.microsoft.com/codeofconduct/ -->
<!-- 🕮 Read our guide about submitting issues: https://github.com/microsoft/vscode/wiki/Submitting-Bugs-and-Suggestions -->
<!-- 🔎 Search existing issues to avoid creating duplicates. -->
<!-- 🧪 Test using the latest Insiders build to see if your issue has already been fixed: https://code.visualstudio.com/insiders/ -->
<!-- 💡 Instead of creating your report here, use 'Report Issue' from the 'Help' menu in VS Code to pre-fill useful information. -->
<!-- 🔧 Launch with code --disable-extensions to check. -->
Does this issue occur when all extensions are disabled?: Yes
- VS Code Version: 1.139.0-insider (4dbe1643e6189ba7b1bbe542cc0e56a94d9ff132)
- OS Version: macOS 14.2 (23C64)
Steps to Reproduce:
- Open anything that resolves a menu through
IMenuService— a tree view row's inline actions, or any context menu. - Profile the renderer. Both
MenuInfoSnapshot's constructor andMenuInfo's constructor appear, each performing a fullMenuRegistry.getMenuItemscopy + sort + context-key collection for the sameMenuId.
The problem
MenuInfoSnapshot's constructor calls this.refresh() (src/vs/platform/actions/common/menuService.ts:175). MenuInfo extends it and calls this.refresh() again immediately after super() (menuService.ts:266):
class MenuInfoSnapshot {
constructor(_id, _collectContextKeysForSubmenus) { this.refresh(); }
}
class MenuInfo extends MenuInfoSnapshot {
constructor(...) { super(_id, _collectContextKeysForSubmenus); this.refresh(); }
}
Because _sort is virtually dispatched, the base constructor's refresh() resolves this._sort to MenuInfo.prototype._sort (menuService.ts:314) rather than the no-op base implementation at menuService.ts:225 — so both passes perform the full sort, not just the cheaper collection walk. The first pass's _menuGroups is then discarded by the second.
This looks like a regression from 603b0ee03b8 (#219964, "Don't listen on menu changed, method 2"), which extracted MenuInfoSnapshot, moved refresh() into the base constructor, and left the derived call in place.
Impact
In a CPU profile of a tree-view interaction on a workspace with many view/item/context contributions, MenuInfoSnapshot's constructor accounts for 7,381ms (35.3%) of a single 20,904ms renderer task — all of it thrown away. MenuInfo's constructor totals 14,268ms (68.3%).
Full profile and surrounding context in #336496.
Suggested fix
Remove the redundant this.refresh() at menuService.ts:266.
Comments Threads Pending Resolution
## Copilot review overview
### 🟢 Approval recommended
The focused optimization preserves menu behavior and has comprehensive targeted tests.
**Review effort:** Balanced
**Findings:** None
What changed in this PR
Optimizes one-shot menu action resolution while preserving current menu behavior.
**Changes:**
- Resolves and filters one-shot actions without creating persistent menus.
- Shares action construction and normalizes default-group sorting.
- Removes redundant refresh work and adds regression coverage.
| File | Description |
| ---- | ----------- |
| src/vs/platform/actions/test/common/menuService.test.ts | Adds performance and behavior regression tests. |
| src/vs/platform/actions/common/menuService.ts | Implements optimized menu resolution and shared action construction. |
| src/vs/platform/actions/common/actions.ts | Updates the API documentation. |
---
💡 Add a `code-review` agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Resolved Comment Threads
No resolved comments have been left on this PR.