Micro performance prompt and improvements for Map usage in src/vs/base

.github/prompts/micro-perf.prompt.md

@@ -0,0 +1,26 @@
+---
+agent: agent
+description: 'Optimize code performance'
+tools: ['edit', 'search', 'new', 'runCommands', 'runTasks', 'usages', 'vscodeAPI', 'problems', 'changes', 'testFailure', 'openSimpleBrowser', 'fetch', 'githubRepo', 'extensions', 'todos', 'runTests']
+---
+# Role
+
+You are an expert performance engineer.
+
+## Instructions
+
+Review the attached file and find all publicly exported class or functions.
+Optimize performance of all exported definitions.
+If the user provided explicit list of classes or functions to optimize, scope your work only to those definitions.
+
+## Guidelines
+
+1. Make sure to analyze usage and calling patterns for each function you optimize.
+2. When you need to change a function or a class, add optimized version of it immediately below the existing definition instead of changing the original.
+3. Optimized function or class name should have the same name as original with '_new' suffix.
+4. Create a file with '.<your-model-name>.perf.js' suffix with perf tests. For example if you are using model 'Foo' and optimizing file name utils.ts, you will create file named 'utils.foo.perf.js'.
+5. **IMPORTANT**: You should use ESM format for the perf test files (i.e. use 'import' instead of 'require').
+5. The perf tests should contain comprehensive perf tests covering identified scenarios and common cases, and comparing old and new implementations.
+6. The results of perf tests and your summary should be placed in another file with '.<your-model-name>.perf.md' suffix, for example 'utils.foo.perf.md'.
+7. The results file must include section per optimized definition with a table with comparison of old vs new implementations with speedup ratios and analysis of results.
+8. At the end ask the user if they want to apply the changes and if the answer is yes, replace original implementations with optimized versions but only in cases where there are significant perf gains and no serious regressions. Revert any other changes to the original code.

src/vs/base/browser/ui/list/rowCache.ts

@@ -33,10 +33,7 @@ export class RowCache<T> implements IDisposable {
 
 		let isStale = false;
 		if (result) {
-			isStale = this.transactionNodesPendingRemoval.has(result.domNode);
-			if (isStale) {
-				this.transactionNodesPendingRemoval.delete(result.domNode);
-			}
+			isStale = this.transactionNodesPendingRemoval.delete(result.domNode);
 		} else {
 			const domNode = $('.monaco-list-row');
 			const renderer = this.getRenderer(templateId);

src/vs/base/browser/ui/menu/menu.ts

@@ -140,9 +140,9 @@ export class Menu extends ActionBar {
 		if (options.enableMnemonics) {
 			this._register(addDisposableListener(menuElement, EventType.KEY_DOWN, (e) => {
 				const key = e.key.toLocaleLowerCase();
-				if (this.mnemonics.has(key)) {
+				const actions = this.mnemonics.get(key);
+				if (actions !== undefined) {
 					EventHelper.stop(e, true);
-					const actions = this.mnemonics.get(key)!;
 
 					if (actions.length === 1) {
 						if (actions[0] instanceof SubmenuMenuActionViewItem && actions[0].container) {
@@ -398,14 +398,12 @@ export class Menu extends ActionBar {
 			if (options.enableMnemonics) {
 				const mnemonic = menuActionViewItem.getMnemonic();
 				if (mnemonic && menuActionViewItem.isEnabled()) {
-					let actionViewItems: BaseMenuActionViewItem[] = [];
-					if (this.mnemonics.has(mnemonic)) {
-						actionViewItems = this.mnemonics.get(mnemonic)!;
+					const actionViewItems = this.mnemonics.get(mnemonic);
+					if (actionViewItems !== undefined) {
+						actionViewItems.push(menuActionViewItem);
+					} else {
+						this.mnemonics.set(mnemonic, [menuActionViewItem]);
 					}
-
-					actionViewItems.push(menuActionViewItem);
-
-					this.mnemonics.set(mnemonic, actionViewItems);
 				}
 			}
 
@@ -423,14 +421,12 @@ export class Menu extends ActionBar {
 			if (options.enableMnemonics) {
 				const mnemonic = menuActionViewItem.getMnemonic();
 				if (mnemonic && menuActionViewItem.isEnabled()) {
-					let actionViewItems: BaseMenuActionViewItem[] = [];
-					if (this.mnemonics.has(mnemonic)) {
-						actionViewItems = this.mnemonics.get(mnemonic)!;
+					const actionViewItems = this.mnemonics.get(mnemonic);
+					if (actionViewItems !== undefined) {
+						actionViewItems.push(menuActionViewItem);
+					} else {
+						this.mnemonics.set(mnemonic, [menuActionViewItem]);
 					}
-
-					actionViewItems.push(menuActionViewItem);
-
-					this.mnemonics.set(mnemonic, actionViewItems);
 				}
 			}
 

src/vs/base/common/cache.ts

@@ -108,8 +108,9 @@ export class CachedFunction<TArg, TComputed> {
 
 	public get(arg: TArg): TComputed {
 		const key = this._computeKey(arg);
-		if (this._map2.has(key)) {
-			return this._map2.get(key)!;
+		const cached = this._map2.get(key);
+		if (cached !== undefined) {
+			return cached;
 		}
 
 		const value = this._fn(arg);
@@ -142,8 +143,9 @@ export class WeakCachedFunction<TArg, TComputed> {
 
 	public get(arg: TArg): TComputed {
 		const key = this._computeKey(arg) as WeakKey;
-		if (this._map.has(key)) {
-			return this._map.get(key)!;
+		const cached = this._map.get(key);
+		if (cached !== undefined) {
+			return cached;
 		}
 
 		const value = this._fn(arg);

src/vs/base/common/filters.ts

@@ -168,8 +168,9 @@ const alternateCharsCache: Map<number, ArrayLike<number> | undefined> = new Map(
  * @param code The character code to check.
  */
 function getAlternateCodes(code: number): ArrayLike<number> | undefined {
-	if (alternateCharsCache.has(code)) {
-		return alternateCharsCache.get(code);
+	const cached = alternateCharsCache.get(code);
+	if (cached !== undefined) {
+		return cached;
 	}
 
 	// NOTE: This function is written in such a way that it can be extended in

src/vs/base/common/lifecycle.ts

@@ -505,8 +505,7 @@ export class DisposableStore implements IDisposable {
 		if (!o) {
 			return;
 		}
-		if (this._toDispose.has(o)) {
-			this._toDispose.delete(o);
+		if (this._toDispose.delete(o)) {
 			setParentOfDisposable(o, null);
 		}
 	}

src/vs/base/common/map.ts

@@ -862,7 +862,8 @@ export function mapsStrictEqualIgnoreOrder(a: Map<unknown, unknown>, b: Map<unkn
 	}
 
 	for (const [key, value] of a) {
-		if (!b.has(key) || b.get(key) !== value) {
+		const bValue = b.get(key);
+		if (bValue === undefined || bValue !== value) {
 			return false;
 		}
 	}
@@ -894,10 +895,12 @@ export class NKeyMap<TValue, TKeys extends (string | boolean | number)[]> {
 	public set(value: TValue, ...keys: [...TKeys]): void {
 		let currentMap = this._data;
 		for (let i = 0; i < keys.length - 1; i++) {
-			if (!currentMap.has(keys[i])) {
-				currentMap.set(keys[i], new Map());
+			let nextMap = currentMap.get(keys[i]);
+			if (nextMap === undefined) {
+				nextMap = new Map();
+				currentMap.set(keys[i], nextMap);
 			}
-			currentMap = currentMap.get(keys[i]);
+			currentMap = nextMap;
 		}
 		currentMap.set(keys[keys.length - 1], value);
 	}
@@ -905,10 +908,11 @@ export class NKeyMap<TValue, TKeys extends (string | boolean | number)[]> {
 	public get(...keys: [...TKeys]): TValue | undefined {
 		let currentMap = this._data;
 		for (let i = 0; i < keys.length - 1; i++) {
-			if (!currentMap.has(keys[i])) {
+			const nextMap = currentMap.get(keys[i]);
+			if (nextMap === undefined) {
 				return undefined;
 			}
-			currentMap = currentMap.get(keys[i]);
+			currentMap = nextMap;
 		}
 		return currentMap.get(keys[keys.length - 1]);
 	}

src/vs/base/common/observableInternal/map.ts

@@ -27,9 +27,8 @@ export class ObservableMap<K, V> implements Map<K, V> {
 	}
 
 	set(key: K, value: V, tx?: ITransaction): this {
-		const hadKey = this._data.has(key);
 		const oldValue = this._data.get(key);
-		if (!hadKey || oldValue !== value) {
+		if (oldValue === undefined || oldValue !== value) {
 			this._data.set(key, value);
 			this._obs.set(this, tx);
 		}

src/vs/base/common/observableInternal/observables/derivedImpl.ts

@@ -381,9 +381,7 @@ export class Derived<T, TChangeSummary = any, TChange = void> extends BaseObserv
 		super.addObserver(observer);
 
 		if (shouldCallBeginUpdate) {
-			if (this._removedObserverToCallEndUpdateOn && this._removedObserverToCallEndUpdateOn.has(observer)) {
-				this._removedObserverToCallEndUpdateOn.delete(observer);
-			} else {
+			if (!this._removedObserverToCallEndUpdateOn?.delete(observer)) {
 				observer.beginUpdate(this);
 			}
 		}

src/vs/base/common/paging.ts

@@ -258,9 +258,7 @@ export class PageIteratorPager<T> implements IPager<T> {
 			}
 			return this.cachedPages[pageIndex];
 		} finally {
-			if (this.pendingRequests.has(pageIndex)) {
-				this.pendingRequests.delete(pageIndex);
-			}
+			this.pendingRequests.delete(pageIndex);
 		}
 	}
 

src/vs/base/common/worker/webWorker.ts

@@ -243,19 +243,21 @@ class WebWorkerProtocol {
 	}
 
 	private _handleEventMessage(msg: EventMessage): void {
-		if (!this._pendingEmitters.has(msg.req)) {
+		const emitter = this._pendingEmitters.get(msg.req);
+		if (emitter === undefined) {
 			console.warn('Got event for unknown req');
 			return;
 		}
-		this._pendingEmitters.get(msg.req)!.fire(msg.event);
+		emitter.fire(msg.event);
 	}
 
 	private _handleUnsubscribeEventMessage(msg: UnsubscribeEventMessage): void {
-		if (!this._pendingEvents.has(msg.req)) {
+		const event = this._pendingEvents.get(msg.req);
+		if (event === undefined) {
 			console.warn('Got unsubscribe for unknown req');
 			return;
 		}
-		this._pendingEvents.get(msg.req)!.dispose();
+		event.dispose();
 		this._pendingEvents.delete(msg.req);
 	}
 
@@ -399,11 +401,12 @@ export class WebWorkerClient<W extends object> extends Disposable implements IWe
 	}
 
 	public getChannel<T extends object>(channel: string): Proxied<T> {
-		if (!this._remoteChannels.has(channel)) {
-			const inst = this._protocol.createProxyToRemoteChannel(channel, async () => { await this._onModuleLoaded; });
+		let inst = this._remoteChannels.get(channel);
+		if (inst === undefined) {
+			inst = this._protocol.createProxyToRemoteChannel(channel, async () => { await this._onModuleLoaded; });
 			this._remoteChannels.set(channel, inst);
 		}
-		return this._remoteChannels.get(channel) as Proxied<T>;
+		return inst as Proxied<T>;
 	}
 
 	private _onError(message: string, error?: unknown): void {
@@ -511,11 +514,12 @@ export class WebWorkerServer<T extends IWebWorkerServerRequestHandler> implement
 	}
 
 	public getChannel<T extends object>(channel: string): Proxied<T> {
-		if (!this._remoteChannels.has(channel)) {
-			const inst = this._protocol.createProxyToRemoteChannel(channel);
+		let inst = this._remoteChannels.get(channel);
+		if (inst === undefined) {
+			inst = this._protocol.createProxyToRemoteChannel(channel);
 			this._remoteChannels.set(channel, inst);
 		}
-		return this._remoteChannels.get(channel) as Proxied<T>;
+		return inst as Proxied<T>;
 	}
 
 	private async initialize(workerId: number): Promise<void> {

src/vs/base/parts/ipc/common/ipc.ts

@@ -1190,8 +1190,9 @@ export namespace ProxyChannel {
 				if (typeof propKey === 'string') {
 
 					// Check for predefined values
-					if (options?.properties?.has(propKey)) {
-						return options.properties.get(propKey);
+					const predefinedValue = options?.properties?.get(propKey);
+					if (predefinedValue !== undefined) {
+						return predefinedValue;
 					}
 
 					// Dynamic Event