microsoft / microsoft/vscode

Perf: per-row getMenuActions is uncached; tree find widget turns every update into a full list rebuild (20s freeze)

Open
#336,496 0 comments 0 reactions 1 assignee Claimed by @benibenj View on GitHub
Dominant language
TypeScript
Stars
193k
Forks
42.4k
PR merge metrics
PR metrics pending

Description

Does this issue occur when all extensions are disabled?: No — the two core defects below are always present, but they are only expensive once enough menu items are registered. See "On the extensions question".

- VS Code Version: 1.139.0-insider (4dbe1643e6189ba7b1bbe542cc0e56a94d9ff132)
- OS Version: macOS 14.2 (23C64)

Steps to Reproduce:

1. Install any extension contributing a large number of `view/item/context` menu items, and open one of its tree views with a few hundred rows.
2. Open the tree's find widget and type any pattern (reproduces in both highlight and filter mode).
3. Cause the view to update — e.g. delete an item, so the extension fires `onDidChangeTreeData`.
4. The renderer blocks for ~20s. With the find widget closed, the identical update is instant.

Measured **INP 18,663ms**, CLS 0.24, in a single 20,904ms `Run microtasks` task.

---

## 1. `getMenuActions` is uncached and re-sorts the whole menu on every call

`MenuService.getMenuActions` (`src/vs/platform/actions/common/menuService.ts:38`) constructs a fresh `MenuImpl` on every call, with no caching:

```ts
getMenuActions(id, contextKeyService, options) {
const menu = new MenuImpl(id, this._hiddenStates, { ... }, this._commandService, this._keybindingService, contextKeyService);
const actions = menu.getActions(options);
menu.dispose();
return actions;
}
```

`MenuRegistry.getMenuItems` (`src/vs/platform/actions/common/actions.ts:531`) returns a fresh array copy each call, which `_sort` (`menuService.ts:314`) then re-sorts with a comparator that bottoms out in `localeCompare` (`_compareTitles`, `menuService.ts:362`).

In tree views this runs **twice per rendered row**:

- `TreeRenderer.renderElement` → `TreeMenus.getResourceActions` (`src/vs/workbench/browser/parts/views/treeView.ts`)
- `accessibilityProvider.getAriaLabel` in the same file, invoked per row by `AccessibiltyRenderer.renderElement` (`src/vs/base/browser/ui/list/listWidget.ts:1314`)

Bottom-up from the profile (total / self):

| function | total |
|---|---|
| `constructor` `menuService.ts:266` (`MenuInfo`) | 14,268ms / **68.3%** |
| `constructor` `menuService.ts:175` (`MenuInfoSnapshot`) | 7,381ms / 35.3% |
| `refresh` `menuService.ts:198` | 13,400ms / 64.1% |
| `_sort` `menuService.ts:314` | 9,240ms / 44.2% (4,070 self) |
| `_compareMenuItems` `menuService.ts:318` | 5,210ms (3,917 self) |
| `_fillInKbExprKeys` `menuService.ts:254` | 3,057ms / 14.6% |
| `contextMatchesRules` `contextKeyService.ts:607` | 3,120ms / 14.9% |
| `_compareTitles` `menuService.ts:362` | 1,220 self (`localeCompare`) |

The 35.3% attributed to `MenuInfoSnapshot`'s constructor is entirely redundant work — filed separately as #336495.

The grouped/sorted structure depends only on the `MenuRegistry` contents for a given `MenuId`, not on the context key service. Only `createActionGroups`'s `contextMatchesRules` pass genuinely needs per-element context, so this looks cacheable.

## 2. The tree find widget turns every model update into a full list rebuild

This is what escalates the above from "a cost per newly rendered row" to "a cost for every row in the viewport, on every update".

`FindController`'s `onDidChangeModel` handler (`src/vs/base/browser/ui/tree/abstractTree.ts:1195`) calls `tree.refilter()` synchronously on every model splice while the widget is open with a non-empty pattern — in highlight mode as well as filter mode:

```ts
this.disposables.add(this.tree.onDidChangeModel(() => {
if (!this.isOpened()) { return; }
if (this.pattern.length !== 0) { this.tree.refilter(); }
this.render();
}));
```

`IndexTreeModel.refilter()` (`indexTreeModel.ts:476`) then emits `{ start: 0, deleteCount: everything }`, which takes the "discard the whole items array and RangeMap" branch in `listView.ts:692` and re-renders every row in the render range. What would otherwise be a targeted `view.splice(listIndex, small, small)` becomes a whole-viewport re-render.

`IndexTreeModel` already coalesces refilters through `refilterDelayer` for exactly this reason (see #135941); the `FindController` path bypasses it by calling `refilter()` synchronously.

Call tree:

```
Run microtasks 20,904.1ms (100%)
refreshAndRenderNode asyncDataTree.ts:1085 20,630.5ms
render → setChildren → splice → spliceSimple 20,629.9ms
fire (onDidSpliceModel)
(anonymous) abstractTree.ts:1195 20,553.7ms
refilter abstractTree.ts:3166 20,549.8ms
refilter indexTreeModel.ts:476
fire event.ts:1411 20,539.6ms ← view.splice(0, all, all)
updateNodeAfterFilterChange 9.7ms
render abstractTree.ts:1260 3.8ms
```

Note the filtering itself is not the cost: `updateNodeAfterFilterChange` is **9.7ms (0.05%)**. Essentially all of the time is the re-render it triggers.

## On the extensions question

Both defects are in core and are present with extensions disabled — the uncached per-call `MenuImpl` construction, and the whole-list rebuild per splice. They simply aren't expensive until enough menu items are registered for the per-row sort to matter, and the cost is linear in the total number of registered `view/item/context` items.

Extension bisect will point at whichever installed extension contributes the most menu items (in my case GitLens, with 875 `view/item/context` entries), but that extension is using a documented, supported contribution point and is not doing anything unusual. Any user with a handful of menu-contributing extensions pays a proportional share of this on every tree row render.

## Suggested fixes, in order of leverage

1. #336495 — remove the redundant `MenuInfo` refresh (~35% of this profile).
2. Cache the sorted/grouped `_menuGroups` per `MenuId`, invalidated on `MenuRegistry.onDidChangeMenu`.
3. In `treeView.ts`, memoize `TreeMenus.getResourceActions` on `element.contextValue` — the overlay is literally `{ view, viewItem }`, and a typical tree view has a handful of distinct `contextValue`s across hundreds of rows. This also collapses the double call per row.
4. Coalesce `FindController`'s refilter through the existing `refilterDelayer`, or have `refilter()` emit a narrower splice.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.