perf(plugins): index contributed keybindings by command (#20241)

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 <baseline-ref>')
+}
+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)
+)

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}`, {

src/main/plugins/plugin-command-registry.ts

@@ -124,6 +124,16 @@ function registrationsForManifest(
   pluginKey: string,
   manifest: PluginManifest
 ): PluginCommandRegistration[] {
+  const bindingsByCommand = new Map<string, PluginKeybindingContribution[]>()
+  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'
+  }))
 }