MenuService: reduce menu rebuild overhead through caching

[n-gist]

Partially addresses #324355

Overview

This PR reduces MenuService overhead during TreeView scrolling and other components item rendering by eliminating repeated work performed while menus are rebuilt for visible items.

Performance investigation #324355 showed that menu processing is executed multiple times for the same element during a single rendering cycle, causing identical menu structures to be repeatedly sorted and analyzed even though neither menu registrations nor context key expressions change during scrolling. This PR focuses on removing that redundant work.

• _sort
• _collectContextKeysAndSubmenuIds

was responsible for a substantial portion of the frame time. In addition, context key expression processing continuously allocated new arrays during menu reconstruction.

The changes in this PR focus on removing these repeated computations and allocations while preserving correctness through immutability guarantees and cache-safe data structures.

Changes (per commit)

1. MenuInfo: Remove duplicate refresh during initialization

MenuInfo extends MenuInfoSnapshot, and both constructors ended up triggering refresh() during initialization.

This change removes the duplicate refresh call, avoiding redundant menu processing during construction.

2. Make IMenuItem and ISubmenuItem properties immutable

Caching sorted menu structures requires stable sorting inputs.

The properties involved in menu ordering were made readonly to guarantee sorting cache correctness. More properties could potentially be made immutable in the future, but this PR limits changes to the fields required for sorting stability. For example, the when property is still constructed and modified in multiple places before command registration and would require a broader refactoring before the entire interface can become immutable.

During this work, ExternalTerminalContribution was identified as the only place in the codebase mutating a registered menu command title. The mutation was replaced with command re-registration, preserving immutability assumptions.

This also fixes an existing issue where the terminal command title could switch to the Windows-specific variant but never switch back when the configuration changed.

3. MenuRegistry: cache stable readonly menu item arrays

MenuRegistry.getMenuItems() previously produced new array instances even when menu contents remained unchanged.

This change introduces stable readonly arrays that are reused while the underlying menu structure remains unchanged.

Since menu registrations typically change very infrequently compared to how often menus are queried, reusing stable arrays is beneficial on its own and also enables additional optimizations built on top of array identity.

4. MenuInfo: cache sorted menu item arrays

As of the previous change, MenuRegistry now returns stable menu item arrays.

This makes it possible to cache MenuInfo._sort results and reuse them whenever the source menu array instance is unchanged, relying on the fact that all properties participating in sorting are now immutable.

This removes repeated sorting work and the associated allocations during menu reconstruction.

Caching was initially implemented at the MenuInfo instance level, but this proved ineffective because MenuInfo instances themselves are not reused. As a result, a global WeakMap cache keyed by the stable source array identity was adopted as the most effective approach.

5. MenuService: cache ContextKeyExpression.keys() results

Profiling showed that ContextKeyExpression.keys() is the primary contributor to the cost of MenuInfoSnapshot._collectContextKeysAndSubmenuIds.

Every call recursively created new arrays even though context key expressions are immutable and frequently reused.

All expression implementations were reviewed to verify that the contents and ordering of keys() results remain stable for the lifetime of an expression instance.

While expression classes could potentially cache their own keys() results internally, this PR intentionally avoids changing expression implementations and instead introduces caching at the MenuService level using a WeakMap.

Benefits:

• Eliminates repeated keys() allocations
• Removes most keys() processing from performance traces
• Reduces garbage collection pressure
• Further improves menu refresh performance after sorting costs have been removed

Notes

The optimizations intentionally rely on object identity and immutability guarantees rather than invalidation logic.

Where caching was introduced, prerequisites were implemented and inspected first to guarantee correctness and prevent stale results.

Performance Results

Tests were performed incrementally after each optimization commit by rapidly scrolling up and down for approximately 10 seconds in an OSS debug build.

Measurement code

Counts renderElement calls and outputs result on 2 seconds of idling.

export class ListView<T> implements IListView<T> {
	private insertItemInDOM(index: number, row?: IRow): void {

		// ...

		const renderer = this.renderers.get(item.templateId);

		if (!renderer) {
			throw new Error(`No renderer found for template id ${item.templateId}`);
		} else {
			perfTickStart();
			try {
				renderer.renderElement(item.element, index, item.row.templateData, { height: item.size });
			} finally {
				perfTickEnd();
			}
		}

		// ...

	}
}

let measureTimeout: TimeoutHandle | undefined;
let measureStart = 0;
let measureTickStart = 0;
let measureCount = 0;
let measureLoad = 0;
let measureLastTickEnd = 0;

const perfTickStart = () => {
	if (measureTimeout === undefined) {
		measureTimeout = setTimeout(perfStop, 2000);

		measureStart = performance.now();
		measureTickStart = measureStart;
		measureCount = 0;
		measureLoad = 0;
	} else {
		clearTimeout(measureTimeout);
		measureTimeout = setTimeout(perfStop, 2000);
		measureTickStart = performance.now();
	}
};

const perfTickEnd = () => {
	const now = performance.now();
	measureLoad += now - measureTickStart;
	measureCount++;
	measureLastTickEnd = now;
};

const perfStop = () => {
	measureTimeout = undefined;

	const duration = measureLastTickEnd - measureStart;
	if (measureCount > 0 && duration > 0) {
		const loadPercent = (measureLoad / duration) * 100;
		console.log(`renderElement: calls=${measureCount}, avg=${(measureLoad / measureCount).toFixed(3)}ms, total ${measureLoad.toFixed(1)}ms, duration=${duration.toFixed(1)}ms, utilization=${loadPercent.toFixed(1)}%`);
	}
};

Measurement results

0. Baseline
renderElement: calls=549, avg=3.964ms, total 2176.1ms, duration=8820.6ms, utilization=24.7%
renderElement: calls=563, avg=3.645ms, total 2052.0ms, duration=9847.7ms, utilization=20.8%
renderElement: calls=466, avg=3.925ms, total 1828.9ms, duration=8901.2ms, utilization=20.5%


1. Remove duplicate refresh
renderElement: calls=493, avg=2.969ms, total 1463.5ms, duration=8202.1ms, utilization=17.8%
renderElement: calls=552, avg=2.918ms, total 1610.9ms, duration=9727.0ms, utilization=16.6%
renderElement: calls=538, avg=2.916ms, total 1568.7ms, duration=8983.2ms, utilization=17.5%

Approximately 24% average renderElement time reduction.


2. Immutability changes

No measurable performance impact by themselves.
These changes were introduced to guarantee cache correctness for subsequent optimizations.


3. Cache stable menu item arrays in MenuRegistry
renderElement: calls=612, avg=2.787ms, total 1705.4ms, duration=9362.6ms, utilization=18.2%
renderElement: calls=620, avg=2.861ms, total 1773.8ms, duration=9345.3ms, utilization=19.0%
renderElement: calls=555, avg=2.954ms, total 1639.3ms, duration=8371.2ms, utilization=19.6%

No significant average renderElement improvement was observed directly, but the number of completed render calls
increased noticeably. This is likely a consequence of reduced allocation pressure and less GC interference.


### 4. Cache MenuInfo sorting results
renderElement: calls=611, avg=2.736ms, total 1671.8ms, duration=9455.5ms, utilization=17.7%
renderElement: calls=618, avg=2.765ms, total 1708.8ms, duration=9388.7ms, utilization=18.2%
renderElement: calls=676, avg=2.703ms, total 1826.9ms, duration=9644.7ms, utilization=18.9%

Total average improvement versus baseline: approximately 29%.


### 5. Cache ContextKeyExpression.keys() results
renderElement: calls=706, avg=2.349ms, total 1658.4ms, duration=9397.7ms, utilization=17.6%
renderElement: calls=727, avg=2.344ms, total 1704.1ms, duration=9728.9ms, utilization=17.5%
renderElement: calls=752, avg=2.367ms, total 1780.3ms, duration=9824.9ms, utilization=18.1%

Both render call count and average renderElement duration improved. Profiling also showed that
ContextKeyExpression.keys() almost completely disappeared from the relevant performance traces
after this change. Total average improvement versus baseline: approximately 35-40%.

Summary

Average renderElement duration:

• Baseline: approximately 3.8–4.0 ms
• After all optimizations: approximately 2.3–2.4 ms

This represents roughly a 35–40% reduction in renderElement execution time during scrolling, and likely benefits other menu-heavy scenarios as well.

Notes

The OSS build does not exhibit the same severity of performance issues described in the #324355. In particular, _sort is not always visible as a significant contributor in OSS traces, even before these optimizations, so commit 4 may be somewhat underrepresented in the measurements above.

However, in the user setup that motivated the investigation, animation frame execution could reach 100–200 ms, with _sort and _collectContextKeysAndSubmenuIds together contributing up to approximately 65% of the total frame cost.

Because these optimizations directly target those hot paths and eliminate a large amount of repeated work and allocations, the expected impact should scale more favorably in scenarios similar to the linked issue than is reflected by the OSS measurements alone.

[Copilot]

Pull request overview

This PR reduces MenuService/MenuRegistry overhead during frequent menu rebuilds (e.g. TreeView scrolling) by eliminating redundant sorting, context-key collection, and array allocations. It partially addresses #324355. The approach layers several identity/immutability-based caches on top of the menu infrastructure and, as a prerequisite, tightens the immutability contracts of menu item interfaces.

Changes:

• Caches stable, frozen menu-item arrays in MenuRegistry.getMenuItems() (invalidated on registration changes), and caches _sort results and ContextKeyExpression.keys() results in MenuInfoSnapshot/MenuInfo via WeakMaps keyed by object identity.
• Removes the duplicate refresh() in the MenuInfo constructor (the base MenuInfoSnapshot already refreshes).
• Makes sort-relevant menu properties readonly (command, group, order, title, ILocalizedString.value/original) to guarantee cache correctness, and refactors ExternalTerminalContribution to re-register menu items instead of mutating a registered command title (also fixing a Windows-variant that never switched back).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/vs/platform/actions/common/menuService.ts	Adds context-key and sorted-menu-item WeakMap caches; removes duplicate refresh(); _sort now returns/accepts readonly arrays.
src/vs/platform/actions/common/actions.ts	Caches frozen getMenuItems results with proper invalidation; makes sort-relevant IMenuItem/ISubmenuItem props readonly.
src/vs/platform/action/common/action.ts	Makes ILocalizedString.value/original and ICommandAction.title readonly.
src/vs/workbench/contrib/externalTerminal/browser/externalTerminal.contribution.ts	Replaces title mutation with menu re-registration keyed by a variant enum; fixes non-reverting Windows title.
src/vs/workbench/services/actions/common/menusExtensionPoint.ts	Refactors item construction to compute group/order/when up-front; behavior-equivalent.