perf: avoid file-to-file scans when selecting deletion roots

This commit is contained in:
Neil
2026-09-05 14:49:00 -07:00
parent d8c4c83063
commit 1a26c85245
3 changed files with 137 additions and 4 deletions
@@ -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))
@@ -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', () => {
@@ -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))
)
}