From 39767f1459ef48a1a9d7df3a01e42671a48fa9b5 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Wed, 19 Nov 2025 11:02:25 -0800 Subject: [PATCH] Micro performance prompt and improvements for Map usage in src/vs/base --- .github/prompts/micro-perf.prompt.md | 26 +++++++++++++++++ src/vs/base/browser/ui/list/rowCache.ts | 5 +--- src/vs/base/browser/ui/menu/menu.ts | 28 ++++++++----------- src/vs/base/common/cache.ts | 10 ++++--- src/vs/base/common/filters.ts | 5 ++-- src/vs/base/common/lifecycle.ts | 3 +- src/vs/base/common/map.ts | 16 +++++++---- src/vs/base/common/observableInternal/map.ts | 3 +- .../observables/derivedImpl.ts | 4 +-- src/vs/base/common/paging.ts | 4 +-- src/vs/base/common/worker/webWorker.ts | 24 +++++++++------- src/vs/base/parts/ipc/common/ipc.ts | 5 ++-- 12 files changed, 79 insertions(+), 54 deletions(-) create mode 100644 .github/prompts/micro-perf.prompt.md diff --git a/.github/prompts/micro-perf.prompt.md b/.github/prompts/micro-perf.prompt.md new file mode 100644 index 00000000000..17eb3d6800e --- /dev/null +++ b/.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 '..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 '..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. diff --git a/src/vs/base/browser/ui/list/rowCache.ts b/src/vs/base/browser/ui/list/rowCache.ts index 4ec97d40923..b1d60d1fd45 100644 --- a/src/vs/base/browser/ui/list/rowCache.ts +++ b/src/vs/base/browser/ui/list/rowCache.ts @@ -33,10 +33,7 @@ export class RowCache 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); diff --git a/src/vs/base/browser/ui/menu/menu.ts b/src/vs/base/browser/ui/menu/menu.ts index 402ba662005..f48c4488073 100644 --- a/src/vs/base/browser/ui/menu/menu.ts +++ b/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); } } diff --git a/src/vs/base/common/cache.ts b/src/vs/base/common/cache.ts index 2e4e17c6295..e8fab592c3c 100644 --- a/src/vs/base/common/cache.ts +++ b/src/vs/base/common/cache.ts @@ -108,8 +108,9 @@ export class CachedFunction { 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 { 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); diff --git a/src/vs/base/common/filters.ts b/src/vs/base/common/filters.ts index fd159b40ab4..2cb532e06ac 100644 --- a/src/vs/base/common/filters.ts +++ b/src/vs/base/common/filters.ts @@ -168,8 +168,9 @@ const alternateCharsCache: Map | undefined> = new Map( * @param code The character code to check. */ function getAlternateCodes(code: number): ArrayLike | 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 diff --git a/src/vs/base/common/lifecycle.ts b/src/vs/base/common/lifecycle.ts index f78aca05b53..2345bfd7028 100644 --- a/src/vs/base/common/lifecycle.ts +++ b/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); } } diff --git a/src/vs/base/common/map.ts b/src/vs/base/common/map.ts index 0eb115b0df0..e3b59bed0c3 100644 --- a/src/vs/base/common/map.ts +++ b/src/vs/base/common/map.ts @@ -862,7 +862,8 @@ export function mapsStrictEqualIgnoreOrder(a: Map, b: Map { 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 { 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]); } diff --git a/src/vs/base/common/observableInternal/map.ts b/src/vs/base/common/observableInternal/map.ts index 5cd028db280..166c03e02c3 100644 --- a/src/vs/base/common/observableInternal/map.ts +++ b/src/vs/base/common/observableInternal/map.ts @@ -27,9 +27,8 @@ export class ObservableMap implements Map { } 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); } diff --git a/src/vs/base/common/observableInternal/observables/derivedImpl.ts b/src/vs/base/common/observableInternal/observables/derivedImpl.ts index e21e374f2d6..96e37ce40c7 100644 --- a/src/vs/base/common/observableInternal/observables/derivedImpl.ts +++ b/src/vs/base/common/observableInternal/observables/derivedImpl.ts @@ -381,9 +381,7 @@ export class Derived 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); } } diff --git a/src/vs/base/common/paging.ts b/src/vs/base/common/paging.ts index 295600dac63..74123dd25b5 100644 --- a/src/vs/base/common/paging.ts +++ b/src/vs/base/common/paging.ts @@ -258,9 +258,7 @@ export class PageIteratorPager implements IPager { } return this.cachedPages[pageIndex]; } finally { - if (this.pendingRequests.has(pageIndex)) { - this.pendingRequests.delete(pageIndex); - } + this.pendingRequests.delete(pageIndex); } } diff --git a/src/vs/base/common/worker/webWorker.ts b/src/vs/base/common/worker/webWorker.ts index 46856d37583..0ad11b838aa 100644 --- a/src/vs/base/common/worker/webWorker.ts +++ b/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 extends Disposable implements IWe } public getChannel(channel: string): Proxied { - 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; + return inst as Proxied; } private _onError(message: string, error?: unknown): void { @@ -511,11 +514,12 @@ export class WebWorkerServer implement } public getChannel(channel: string): Proxied { - 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; + return inst as Proxied; } private async initialize(workerId: number): Promise { diff --git a/src/vs/base/parts/ipc/common/ipc.ts b/src/vs/base/parts/ipc/common/ipc.ts index 59a4f9f1d99..b3a4f8ece2d 100644 --- a/src/vs/base/parts/ipc/common/ipc.ts +++ b/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