diff --git a/config/scripts/plugin-command-bindings-benchmark.mjs b/config/scripts/plugin-command-bindings-benchmark.mjs new file mode 100644 index 00000000000..58b3dad8cab --- /dev/null +++ b/config/scripts/plugin-command-bindings-benchmark.mjs @@ -0,0 +1,101 @@ +import assert from 'node:assert/strict' +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' +import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs' + +const baseline = process.argv[2] +if (!baseline) { + throw new Error('Usage: node config/scripts/plugin-command-bindings-benchmark.mjs ') +} +async function load(file, contents, name) { + const result = await build({ + stdin: { contents, loader: 'ts', resolveDir: dirname(resolve(file)) }, + bundle: true, + platform: 'node', + format: 'esm', + write: false, + logLevel: 'silent', + tsconfigRaw: {}, + banner: { + js: "import { createRequire as benchmarkRequire } from 'node:module'; import { resolve as benchmarkPath } from 'node:path'; const require = benchmarkRequire(benchmarkPath('package.json'));" + } + }) + return ( + await import( + `data:text/javascript;base64,${Buffer.from(result.outputFiles[0].text).toString('base64')}` + ) + )[name] +} +const file = 'src/main/plugins/plugin-command-registry.ts' +const before = await load( + file, + execFileSync('git', ['show', `${baseline}:${file}`], { encoding: 'utf8' }), + 'PluginCommandRegistry' +) +const after = await load(file, readFileSync(file, 'utf8'), 'PluginCommandRegistry') +const results = [] +for (const count of [1, 16, 64, 256]) { + const plugins = [ + { + pluginKey: 'sample.commands', + manifest: { + contributes: { + commands: Array.from({ length: count }, (_, index) => ({ + id: `command-${index}`, + title: `Command ${index}`, + action: 'view.tasks' + })), + keybindings: Array.from({ length: Math.min(count, 104) }, (_, index) => ({ + command: `command-${index}`, + key: `Mod+${Math.floor(index / 26) & 1 ? 'Alt+' : ''}${Math.floor(index / 26) & 2 ? 'Shift+' : ''}${String.fromCharCode(65 + (index % 26))}` + })) + } + } + } + ] + const arms = { before: new before(), after: new after() } + for (const platform of ['darwin', 'linux', 'win32']) { + for (const arm of Object.values(arms)) { + arm.reconcile(plugins, () => true, {}, platform) + } + const snapshot = (registry) => ({ + active: registry.list(), + previews: plugins.map((plugin) => registry.preview(plugin.pluginKey)), + errors: plugins.map((plugin) => registry.error(plugin.pluginKey)) + }) + assert.deepEqual(snapshot(arms.after), snapshot(arms.before)) + } + const iterations = 100 + function run(arm) { + const start = performance.now() + for (let i = 0; i < iterations; i++) { + arms[arm].reconcile(plugins, () => true, {}, 'linux') + } + return (performance.now() - start) / iterations + } + const samples = { before: [], after: [] } + run('before') + run('after') + for (const pair of buildCounterbalancedSchedule(10, 'before', 'after')) { + for (const arm of pair) { + samples[arm].push(run(arm)) + } + } + function median(values) { + const sorted = [...values].sort((a, b) => a - b) + return (sorted[4] + sorted[5]) / 2 + } + results.push({ + commands: count, + bindings: Math.min(count, 104), + beforeMs: median(samples.before), + afterMs: median(samples.after), + samples + }) +} +console.log( + JSON.stringify({ baseline, node: process.version, platform: process.platform, results }, null, 2) +) diff --git a/src/main/plugins/plugin-command-registry.test.ts b/src/main/plugins/plugin-command-registry.test.ts index d68f31eabbb..3fc2d7f819e 100644 --- a/src/main/plugins/plugin-command-registry.test.ts +++ b/src/main/plugins/plugin-command-registry.test.ts @@ -37,6 +37,47 @@ function commandPlugin( } describe('PluginCommandRegistry', () => { + it('reads binding command IDs once while preserving declaration and binding order', () => { + const commands = Array.from({ length: 256 }, (_, index) => ({ + id: `command-${index}`, + title: `Command ${index}`, + action: 'view.tasks' + })) + const keys = Array.from( + { length: 104 }, + (_, index) => + `Mod+${Math.floor(index / 26) & 1 ? 'Alt+' : ''}${Math.floor(index / 26) & 2 ? 'Shift+' : ''}${String.fromCharCode(65 + (index % 26))}` + ) + // Distinct physical chords, with two bindings belonging to the same command. + const uniqueKeys = [...new Set(keys)] + const plugin = commandPlugin('many-commands', { + commands, + keybindings: uniqueKeys.map((key, index) => ({ command: `command-${index % 32}`, key })) + }) + let reads = 0 + for (const binding of plugin.manifest.contributes.keybindings) { + const command = binding.command + Object.defineProperty(binding, 'command', { + get: () => { + reads++ + return command + } + }) + } + const registry = new PluginCommandRegistry() + registry.reconcile([plugin], () => false) + const preview = registry.preview(plugin.pluginKey) + expect(preview.map((command) => command.id)).toEqual(commands.map((command) => command.id)) + expect(preview[0].keybindings.map((binding) => binding.key)).toEqual( + plugin.manifest.contributes.keybindings + .filter((_, index) => index % 32 === 0) + .map((binding) => binding.key) + ) + expect(preview[255].keybindings).toEqual([]) + expect(registry.list()).toEqual([]) + expect(reads).toBe(uniqueKeys.length) + }) + it('records each conflicting owner once instead of every pair', () => { const plugins = Array.from({ length: 128 }, (_, index) => commandPlugin(`plugin-${index}`, { diff --git a/src/main/plugins/plugin-command-registry.ts b/src/main/plugins/plugin-command-registry.ts index 62c62783375..fc18fa3e093 100644 --- a/src/main/plugins/plugin-command-registry.ts +++ b/src/main/plugins/plugin-command-registry.ts @@ -124,6 +124,16 @@ function registrationsForManifest( pluginKey: string, manifest: PluginManifest ): PluginCommandRegistration[] { + const bindingsByCommand = new Map() + for (const binding of manifest.contributes.keybindings) { + const commandId = binding.command + const bindings = bindingsByCommand.get(commandId) + if (bindings) { + bindings.push(binding) + } else { + bindingsByCommand.set(commandId, [binding]) + } + } return manifest.contributes.commands.map((command) => ({ pluginKey, id: command.id, @@ -133,7 +143,7 @@ function registrationsForManifest( command.action === undefined ? { type: 'worker' as const } : { type: 'built-in' as const, action: command.action as PluginCommandAliasActionId }, - keybindings: keybindingsForCommand(command, manifest.contributes.keybindings) + keybindings: keybindingsForCommand(command, bindingsByCommand.get(command.id) ?? []) })) } @@ -141,10 +151,8 @@ function keybindingsForCommand( command: PluginCommandContribution, keybindings: readonly PluginKeybindingContribution[] ): PluginCommandKeybinding[] { - return keybindings - .filter((keybinding) => keybinding.command === command.id) - .map((keybinding) => ({ - key: keybinding.key, - when: keybinding.when ?? command.context ?? 'global' - })) + return keybindings.map((keybinding) => ({ + key: keybinding.key, + when: keybinding.when ?? command.context ?? 'global' + })) }