diff --git a/config/scripts/skill-source-inventory-benchmark.mjs b/config/scripts/skill-source-inventory-benchmark.mjs new file mode 100644 index 00000000000..0e0f299b45b --- /dev/null +++ b/config/scripts/skill-source-inventory-benchmark.mjs @@ -0,0 +1,149 @@ +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import { performance } from 'node:perf_hooks' +import { transform } from 'esbuild' +import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs' + +const path = 'src/renderer/src/components/skills/skill-source-inventory.ts' +const arms = {} +for (const [name, source] of [ + ['baseline', readFileSync(0, 'utf8')], + ['indexed', readFileSync(path, 'utf8')] +]) { + const { code } = await transform(source, { loader: 'ts', format: 'esm' }) + const loaded = await import(`data:text/javascript;base64,${Buffer.from(code).toString('base64')}`) + assert.equal(typeof loaded.summarizeSkillSources, 'function', 'Pipe the baseline module on stdin') + arms[name] = loaded.summarizeSkillSources +} + +function verify(result) { + const expected = arms.baseline(result) + const actual = arms.indexed(result) + assert.deepEqual(actual, expected) + actual.forEach((entry, index) => assert.equal(entry.source, result.sources[index])) + return expected +} + +let seed = 20260911 +function random(max) { + seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0 + return Math.floor((seed / 0x100000000) * max) +} +const paths = [ + '/home/ada/.agents/skills', + '/repo/.agents/skills', + '/REPO/.agents/skills', + '/remote/folder/.claude/skills', + 'C:\\Users\\Ada\\.codex\\skills', + '\\\\wsl$\\Ubuntu\\home\\ada\\.agents\\skills', + '', + '/not-listed' +] +verify(null) +for (let trial = 0; trial < 5000; trial++) { + const sources = Array.from({ length: random(25) }, (_, index) => + Object.freeze({ + id: `${index}`, + path: paths[random(paths.length - 1)], + exists: Boolean(random(2)), + skippedReason: [undefined, 'missing', 'remote-repo', 'unavailable'][random(4)] + }) + ) + const skills = [] + const count = random(100) + for (let index = 0; index < count; index++) { + skills.push( + skills.length && random(4) === 0 + ? skills[random(skills.length)] + : Object.freeze({ + rootPath: paths[random(paths.length)], + rootPaths: random(3) + ? Object.freeze(Array.from({ length: random(15) }, () => paths[random(paths.length)])) + : undefined + }) + ) + } + verify(Object.freeze({ sources: Object.freeze(sources), skills: Object.freeze(skills) })) +} +console.log( + JSON.stringify({ + differentialCases: 5001, + node: process.version, + platform: process.platform, + arch: process.arch + }) +) + +function workload(sourceCount, skillCount, rootsPerSkill) { + const paths = Array.from( + { length: sourceCount || 1 }, + (_, index) => `/repo-${index}/.agents/skills` + ) + return { + sources: paths.slice(0, sourceCount).map((path, index) => ({ + id: `${index}`, + path, + exists: index % 3 !== 0, + skippedReason: index % 5 ? 'missing' : 'unavailable' + })), + skills: Array.from({ length: skillCount }, (_, index) => ({ + rootPath: paths[index % paths.length], + rootPaths: Array.from( + { length: rootsPerSkill }, + (_, rootIndex) => paths[(index + rootIndex) % paths.length] + ) + })) + } +} + +function median(values) { + const sorted = [...values].sort((a, b) => a - b) + return (sorted[3] + sorted[4]) / 2 +} + +for (const [sourceCount, skillCount, rootsPerSkill] of [ + [0, 1000, 1], + [1, 1000, 0], + [1, 1000, 1], + [17, 0, 0], + [17, 20, 1], + [17, 200, 1], + [24, 1000, 3], + [87, 1000, 3], + [367, 5000, 3], + [17, 200, 17] +]) { + const input = workload(sourceCount, skillCount, rootsPerSkill) + const expected = verify(input) + const samples = { baseline: [], indexed: [] } + const repeats = Math.max( + 5, + Math.floor(200000 / (Math.max(1, sourceCount) * Math.max(1, skillCount))) + ) + for (const run of Object.values(arms)) { + for (let warmup = 0; warmup < Math.min(100, repeats); warmup++) { + run(input) + } + } + for (const pair of buildCounterbalancedSchedule(8, 'baseline', 'indexed')) { + for (const arm of pair) { + const start = performance.now() + let result + for (let repeat = 0; repeat < repeats; repeat++) { + result = arms[arm](input) + } + samples[arm].push((performance.now() - start) / repeats) + assert.deepEqual(result, expected) + } + } + console.log( + JSON.stringify({ + sourceCount, + skillCount, + rootsPerSkill, + medianMs: Object.fromEntries( + Object.entries(samples).map(([arm, values]) => [arm, median(values)]) + ) + }) + ) +} diff --git a/src/renderer/src/components/skills/skill-source-inventory.test.ts b/src/renderer/src/components/skills/skill-source-inventory.test.ts index 67fd41965e8..1d863ea3f6b 100644 --- a/src/renderer/src/components/skills/skill-source-inventory.test.ts +++ b/src/renderer/src/components/skills/skill-source-inventory.test.ts @@ -41,6 +41,61 @@ function result(overrides: Partial = {}): SkillDiscoveryRe } describe('summarizeSkillSources', () => { + it('counts each skill once per root, including a primary root omitted from rootPaths', () => { + const shared = skill({ rootPaths: ['/other', '/other', '/unknown'] }) + const home = source() + const other = source({ path: '/other' }) + const entries = summarizeSkillSources( + result({ + sources: [home, other, other, source({ path: '/OTHER' })], + skills: [shared, shared, skill({ rootPaths: [home.path, home.path] })] + }) + ) + expect(entries.map((entry) => entry.skillCount)).toEqual([3, 2, 2, 0]) + expect(entries[0].source).toBe(home) + expect(entries[1].source).toBe(other) + expect(entries[2].source).toBe(other) + }) + + it('does not scan every skill again for each source', () => { + let rootReads = 0 + const skills = Array.from({ length: 1000 }, () => ({ + ...skill(), + get rootPath() { + rootReads++ + return '/home/dev/.agents/skills' + } + })) + const sources = Array.from({ length: 87 }, (_, index) => source({ id: `${index}` })) + const entries = summarizeSkillSources(result({ skills, sources })) + expect(entries.every((entry) => entry.skillCount === 1000)).toBe(true) + expect(rootReads).toBeLessThanOrEqual(skills.length) + }) + + it('does not inspect skills without sources', () => { + const unused = { + ...skill(), + get rootPath(): string { + throw new Error('No source needs a count') + } + } + expect(summarizeSkillSources(null)).toEqual([]) + expect(summarizeSkillSources(result({ skills: [unused] }))).toEqual([]) + }) + + it('accepts frozen ownership lists and inputs without changing them', () => { + const home = Object.freeze(source()) + const item = skill({ rootPaths: [home.path, '/co-owner', home.path] }) + Object.freeze(item.rootPaths) + Object.freeze(item) + const discovery = result({ sources: [home, source({ path: '/co-owner' })], skills: [item] }) + Object.freeze(discovery.sources) + Object.freeze(discovery.skills) + Object.freeze(discovery) + expect(summarizeSkillSources(discovery).map((entry) => entry.skillCount)).toEqual([1, 1]) + expect(item.rootPaths).toEqual([home.path, '/co-owner', home.path]) + }) + it('counts a symlinked skill under every root that reached it', () => { const shared = source({ id: 'repo', path: '/repo/.agents/skills', sourceKind: 'repo' }) const entries = summarizeSkillSources( diff --git a/src/renderer/src/components/skills/skill-source-inventory.ts b/src/renderer/src/components/skills/skill-source-inventory.ts index 1bedd5fa02b..708334530f1 100644 --- a/src/renderer/src/components/skills/skill-source-inventory.ts +++ b/src/renderer/src/components/skills/skill-source-inventory.ts @@ -13,8 +13,6 @@ export type SkillSourceInventoryEntry = { } function ownsSkill(source: SkillDiscoverySource, skill: DiscoveredSkill): boolean { - // Why: a symlinked skill is deduped to one row but keeps every root that - // reached it, so counting only `rootPath` would zero out the co-owning roots. return skill.rootPath === source.path || (skill.rootPaths?.includes(source.path) ?? false) } @@ -37,12 +35,38 @@ function sourceStatus(source: SkillDiscoverySource): SkillSourceStatus { export function summarizeSkillSources( result: SkillDiscoveryResult | null ): SkillSourceInventoryEntry[] { - if (!result) { + if (!result || result.sources.length === 0) { return [] } + // With no repeated skill traversal, the direct count needs no index. + if (result.sources.length === 1 || result.skills.length === 0) { + return result.sources.map((source) => ({ + source, + skillCount: result.skills.filter((skill) => ownsSkill(source, skill)).length, + status: sourceStatus(source) + })) + } + const counts = new Map( + result.sources.map((source) => [source.path, { count: 0, lastSkillIndex: -1 }]) + ) + const countRoot = (rootPath: string, skillIndex: number): void => { + const count = counts.get(rootPath) + // Symlinked skills can name one owning root more than once. + if (count && count.lastSkillIndex !== skillIndex) { + count.count++ + count.lastSkillIndex = skillIndex + } + } + for (let index = 0; index < result.skills.length; index++) { + const skill = result.skills[index] + countRoot(skill.rootPath, index) + for (const rootPath of skill.rootPaths ?? []) { + countRoot(rootPath, index) + } + } return result.sources.map((source) => ({ source, - skillCount: result.skills.filter((skill) => ownsSkill(source, skill)).length, + skillCount: counts.get(source.path)?.count ?? 0, status: sourceStatus(source) })) }