use MicrotaskEmitter to (drastically) reduce the number of MenuRegistry-change events

Author: jrieken

src/vs/base/common/event.ts

@@ -792,7 +792,7 @@ export class Emitter<T> {
 		if (!this._listeners) {
 			return false;
 		}
-		return (!this._listeners.isEmpty());
+		return !this._listeners.isEmpty();
 	}
 }
 
@@ -994,6 +994,11 @@ export class MicrotaskEmitter<T> extends Emitter<T> {
 		this._mergeFn = options?.merge;
 	}
 	override fire(event: T): void {
+
+		if (!this.hasListeners()) {
+			return;
+		}
+
 		this._queuedEvents.push(event);
 		if (this._queuedEvents.length === 1) {
 			queueMicrotask(() => {

src/vs/platform/actions/common/actions.ts

@@ -5,8 +5,7 @@
 
 import { Action, IAction, Separator, SubmenuAction } from 'vs/base/common/actions';
 import { CSSIcon } from 'vs/base/common/codicons';
-import { Emitter, Event } from 'vs/base/common/event';
-import { Iterable } from 'vs/base/common/iterator';
+import { Event, MicrotaskEmitter } from 'vs/base/common/event';
 import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 import { LinkedList } from 'vs/base/common/linkedList';
 import { ICommandAction, ICommandActionTitle, Icon, ILocalizedString } from 'vs/platform/action/common/action';
@@ -234,12 +233,46 @@ export interface IMenuRegistryChangeEvent {
 	has(id: MenuId): boolean;
 }
 
+class MenuRegistryChangeEvent {
+
+	private static _all = new Map<MenuId, MenuRegistryChangeEvent>();
+
+	static for(id: MenuId): MenuRegistryChangeEvent {
+		let value = this._all.get(id);
+		if (!value) {
+			value = new MenuRegistryChangeEvent(id);
+			this._all.set(id, value);
+		}
+		return value;
+	}
+
+	static merge(events: IMenuRegistryChangeEvent[]): IMenuRegistryChangeEvent {
+		const ids = new Set<MenuId>();
+		for (const item of events) {
+			if (item instanceof MenuRegistryChangeEvent) {
+				ids.add(item.id);
+			}
+		}
+		return ids;
+	}
+
+	readonly has: (id: MenuId) => boolean;
+
+	private constructor(private readonly id: MenuId) {
+		this.has = candidate => candidate === id;
+	}
+}
+
 export interface IMenuRegistry {
 	readonly onDidChangeMenu: Event<IMenuRegistryChangeEvent>;
-	addCommands(newCommands: Iterable<ICommandAction>): IDisposable;
 	addCommand(userCommand: ICommandAction): IDisposable;
 	getCommand(id: string): ICommandAction | undefined;
 	getCommands(): ICommandsMap;
+
+	/**
+	 * @deprecated Use `appendMenuItem` or most likely use `registerAction2` instead. There should be no strong
+	 * reason to use this directly.
+	 */
 	appendMenuItems(items: Iterable<{ id: MenuId; item: IMenuItem | ISubmenuItem }>): IDisposable;
 	appendMenuItem(menu: MenuId, item: IMenuItem | ISubmenuItem): IDisposable;
 	getMenuItems(loc: MenuId): Array<IMenuItem | ISubmenuItem>;
@@ -249,30 +282,19 @@ export const MenuRegistry: IMenuRegistry = new class implements IMenuRegistry {
 
 	private readonly _commands = new Map<string, ICommandAction>();
 	private readonly _menuItems = new Map<MenuId, LinkedList<IMenuItem | ISubmenuItem>>();
-	private readonly _onDidChangeMenu = new Emitter<IMenuRegistryChangeEvent>();
+	private readonly _onDidChangeMenu = new MicrotaskEmitter<IMenuRegistryChangeEvent>({
+		merge: MenuRegistryChangeEvent.merge
+	});
 
 	readonly onDidChangeMenu: Event<IMenuRegistryChangeEvent> = this._onDidChangeMenu.event;
 
 	addCommand(command: ICommandAction): IDisposable {
-		return this.addCommands(Iterable.single(command));
-	}
-
-	private readonly _commandPaletteChangeEvent: IMenuRegistryChangeEvent = {
-		has: id => id === MenuId.CommandPalette
-	};
+		this._commands.set(command.id, command);
+		this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(MenuId.CommandPalette));
 
-	addCommands(commands: Iterable<ICommandAction>): IDisposable {
-		for (const command of commands) {
-			this._commands.set(command.id, command);
-		}
-		this._onDidChangeMenu.fire(this._commandPaletteChangeEvent);
 		return toDisposable(() => {
-			let didChange = false;
-			for (const command of commands) {
-				didChange = this._commands.delete(command.id) || didChange;
-			}
-			if (didChange) {
-				this._onDidChangeMenu.fire(this._commandPaletteChangeEvent);
+			if (this._commands.delete(command.id)) {
+				this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(MenuId.CommandPalette));
 			}
 		});
 	}
@@ -288,35 +310,22 @@ export const MenuRegistry: IMenuRegistry = new class implements IMenuRegistry {
 	}
 
 	appendMenuItem(id: MenuId, item: IMenuItem | ISubmenuItem): IDisposable {
-		return this.appendMenuItems(Iterable.single({ id, item }));
+		let list = this._menuItems.get(id);
+		if (!list) {
+			list = new LinkedList();
+			this._menuItems.set(id, list);
+		}
+		const rm = list.push(item);
+		this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(id));
+		return toDisposable(rm);
 	}
 
 	appendMenuItems(items: Iterable<{ id: MenuId; item: IMenuItem | ISubmenuItem }>): IDisposable {
-
-		const changedIds = new Set<MenuId>();
-		const toRemove = new LinkedList<Function>();
-
+		const result = new DisposableStore();
 		for (const { id, item } of items) {
-			let list = this._menuItems.get(id);
-			if (!list) {
-				list = new LinkedList();
-				this._menuItems.set(id, list);
-			}
-			toRemove.push(list.push(item));
-			changedIds.add(id);
+			result.add(this.appendMenuItem(id, item));
 		}
-
-		this._onDidChangeMenu.fire(changedIds);
-
-		return toDisposable(() => {
-			if (toRemove.size > 0) {
-				for (const fn of toRemove) {
-					fn();
-				}
-				this._onDidChangeMenu.fire(changedIds);
-				toRemove.clear();
-			}
-		});
+		return result;
 	}
 
 	getMenuItems(id: MenuId): Array<IMenuItem | ISubmenuItem> {
@@ -568,7 +577,9 @@ export function registerAction2(ctor: { new(): Action2 }): IDisposable {
 
 	// menu
 	if (Array.isArray(menu)) {
-		disposables.add(MenuRegistry.appendMenuItems(menu.map(item => ({ id: item.id, item: { command, ...item } }))));
+		for (const item of menu) {
+			disposables.add(MenuRegistry.appendMenuItem(item.id, { command, ...item }));
+		}
 
 	} else if (menu) {
 		disposables.add(MenuRegistry.appendMenuItem(menu.id, { command, ...menu }));

src/vs/workbench/services/actions/common/menusExtensionPoint.ts

@@ -17,7 +17,7 @@ import { Iterable } from 'vs/base/common/iterator';
 import { index } from 'vs/base/common/arrays';
 import { isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';
 import { ApiProposalName } from 'vs/workbench/services/extensions/common/extensionsApiProposals';
-import { ILocalizedString, ICommandAction } from 'vs/platform/action/common/action';
+import { ILocalizedString } from 'vs/platform/action/common/action';
 
 interface IAPIMenu {
 	readonly key: string;
@@ -631,7 +631,7 @@ export const commandsExtensionPoint = ExtensionsRegistry.registerExtensionPoint<
 
 commandsExtensionPoint.setHandler(extensions => {
 
-	function handleCommand(userFriendlyCommand: schema.IUserFriendlyCommand, extension: IExtensionPointUser<any>, bucket: ICommandAction[]) {
+	function handleCommand(userFriendlyCommand: schema.IUserFriendlyCommand, extension: IExtensionPointUser<any>) {
 
 		if (!schema.isValidCommand(userFriendlyCommand, extension.collector)) {
 			return;
@@ -655,7 +655,7 @@ commandsExtensionPoint.setHandler(extensions => {
 		if (MenuRegistry.getCommand(command)) {
 			extension.collector.info(localize('dup', "Command `{0}` appears multiple times in the `commands` section.", userFriendlyCommand.command));
 		}
-		bucket.push({
+		_commandRegistrations.add(MenuRegistry.addCommand({
 			id: command,
 			title,
 			source: extension.description.displayName ?? extension.description.name,
@@ -664,24 +664,22 @@ commandsExtensionPoint.setHandler(extensions => {
 			category,
 			precondition: ContextKeyExpr.deserialize(enablement),
 			icon: absoluteIcon
-		});
+		}));
 	}
 
 	// remove all previous command registrations
 	_commandRegistrations.clear();
 
-	const newCommands: ICommandAction[] = [];
 	for (const extension of extensions) {
 		const { value } = extension;
 		if (Array.isArray(value)) {
 			for (const command of value) {
-				handleCommand(command, extension, newCommands);
+				handleCommand(command, extension);
 			}
 		} else {
-			handleCommand(value, extension, newCommands);
+			handleCommand(value, extension);
 		}
 	}
-	_commandRegistrations.add(MenuRegistry.addCommands(newCommands));
 });
 
 interface IRegisteredSubmenu {
@@ -762,8 +760,6 @@ menusExtensionPoint.setHandler(extensions => {
 	_menuRegistrations.clear();
 	_submenuMenuItems.clear();
 
-	const items: { id: MenuId; item: IMenuItem | ISubmenuItem }[] = [];
-
 	for (const extension of extensions) {
 		const { value, collector } = extension;
 
@@ -855,10 +851,8 @@ menusExtensionPoint.setHandler(extensions => {
 				}
 
 				item.when = ContextKeyExpr.deserialize(menuItem.when);
-				items.push({ id: menu.id, item });
+				_menuRegistrations.add(MenuRegistry.appendMenuItem(menu.id, item));
 			}
 		}
 	}
-
-	_menuRegistrations.add(MenuRegistry.appendMenuItems(items));
 });