diff --git a/config/scripts/file-explorer-deletion-roots-benchmark.mjs b/config/scripts/file-explorer-deletion-roots-benchmark.mjs new file mode 100644 index 00000000000..d276964861b --- /dev/null +++ b/config/scripts/file-explorer-deletion-roots-benchmark.mjs @@ -0,0 +1,75 @@ +import assert from 'node:assert/strict' +import { join } from 'node:path' +import { performance } from 'node:perf_hooks' +import { fileURLToPath } from 'node:url' +import { build } from 'esbuild' + +const root = fileURLToPath(new URL('../..', import.meta.url)) +const bundled = await build({ + stdin: { + contents: `export { selectDeletionRoots } from './file-explorer-batch-deletion'; + export { isPathEqualOrDescendant } from './file-explorer-paths';`, + resolveDir: join(root, 'src/renderer/src/components/right-sidebar'), + loader: 'ts' + }, + alias: { '@': join(root, 'src/renderer/src') }, + bundle: true, + platform: 'node', + format: 'esm', + write: false, + logLevel: 'silent' +}) +const { selectDeletionRoots, isPathEqualOrDescendant } = await import( + `data:text/javascript;base64,${Buffer.from(bundled.outputFiles[0].text).toString('base64')}` +) + +// Original production selector; both paths use the same path-comparison implementation. +function original(nodes) { + return nodes.filter( + (n) => + !nodes.some( + (other) => other !== n && other.isDirectory && isPathEqualOrDescendant(n.path, other.path) + ) + ) +} + +function measure(run, nodes) { + for (let index = 0; index < 3; index++) { + run(nodes) + } + const samples = [] + for (let index = 0; index < 11; index++) { + const start = performance.now() + run(nodes) + samples.push(performance.now() - start) + } + return samples.sort((a, b) => a - b)[5] +} + +const results = [] +for (const [fileCount, directoryCount] of [ + [100, 0], + [1000, 0], + [5000, 0], + [5000, 5], + [0, 100] +]) { + const nodes = Array.from({ length: fileCount + directoryCount }, (_, index) => ({ + name: `item-${index}`, + path: `/repo/item-${index}`, + relativePath: `item-${index}`, + isDirectory: index >= fileCount, + depth: 0 + })) + const expected = original(nodes) + const actual = selectDeletionRoots(nodes) + assert.equal(actual.length, expected.length) + actual.forEach((node, index) => assert.equal(node, expected[index])) + results.push({ + fileCount, + directoryCount, + beforeMs: measure(original, nodes), + afterMs: measure(selectDeletionRoots, nodes) + }) +} +console.log(JSON.stringify({ node: process.version, platform: process.platform, results }, null, 2)) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.test.ts index e93928bf4fd..688683bce13 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.test.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.test.ts @@ -37,6 +37,66 @@ describe('selectDeletionRoots', () => { const other = node('/repo/a.ts/impossible-child') expect(selectDeletionRoots([file, other])).toEqual([file, other]) }) + + it('reads directory membership once per selected node, including file-only selections', () => { + let directoryReads = 0 + const nodes = Array.from({ length: 1_000 }, (_, index) => ({ + ...node(`/repo/file-${index}.ts`), + get isDirectory() { + directoryReads += 1 + return false + } + })) + const selected = selectDeletionRoots(nodes) + expect(directoryReads).toBe(nodes.length) + expect(selected).not.toBe(nodes) + selected.forEach((entry, index) => expect(entry).toBe(nodes[index])) + }) + + it('preserves order, node identity and host ownership when children precede parents', () => { + const child = node('/repo/docs/guide.md') + const first = { ...node('/repo/first.ts'), operationOwner: { kind: 'local' as const } } + const parent = { + ...node('/repo/docs', true), + operationOwner: { kind: 'ssh' as const, connectionId: 'ssh-owner' } + } + const last = { + ...node('/repo/last.ts'), + operationOwner: { kind: 'unresolved' as const } + } + const nodes = [child, first, parent, last] + const selected = selectDeletionRoots(nodes) + expect(selected).toEqual([first, parent, last]) + expect(selected[0]).toBe(first) + expect(selected[1]).toBe(parent) + expect(selected[2]).toBe(last) + expect(nodes).toEqual([child, first, parent, last]) + }) + + it('keeps repeated references but excludes distinct directories with the same path', () => { + const dir = node('/repo/docs', true) + expect(selectDeletionRoots([dir, dir])).toEqual([dir, dir]) + expect(selectDeletionRoots([dir, { ...dir }])).toEqual([]) + const file = node('/repo/docs') + expect(selectDeletionRoots([file, dir])).toEqual([dir]) + }) + + it.each([ + ['/repo/docs/', '/repo/docs/guide.md', '/repo/docs-other/guide.md'], + ['C:\\Repo\\Docs\\', 'c:/repo/docs/guide.md', 'C:/Repo/Docs-other/guide.md'], + ['\\\\Server\\Share\\Docs', '//server/share/docs/guide.md', '//server/other/docs/guide.md'] + ])('retains path boundaries for %s', (parentPath, childPath, outsidePath) => { + const parent = node(parentPath, true) + const child = node(childPath) + const outside = node(outsidePath) + expect(selectDeletionRoots([outside, child, parent])).toEqual([outside, parent]) + }) + + it('keeps case-distinct POSIX paths', () => { + const parent = node('/repo/Docs', true) + const outside = node('/repo/docs/guide.md') + expect(selectDeletionRoots([outside, parent])).toEqual([outside, parent]) + }) }) describe('runBatchDeletion', () => { diff --git a/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.ts b/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.ts index 63a5d029739..5f052bec588 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-batch-deletion.ts @@ -5,11 +5,9 @@ import type { TreeNode } from './file-explorer-types' // already removes the child, and issuing both requests races on the // now-missing path and produces spurious errors. export function selectDeletionRoots(nodes: TreeNode[]): TreeNode[] { + const directories = nodes.filter((node) => node.isDirectory) return nodes.filter( - (n) => - !nodes.some( - (other) => other !== n && other.isDirectory && isPathEqualOrDescendant(n.path, other.path) - ) + (n) => !directories.some((other) => other !== n && isPathEqualOrDescendant(n.path, other.path)) ) }