mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
perf(paths): drop the root hoist, land the guards alone
The three in-module guards are the whole win: 5000-op batches, CPU time, median of 9 --- normalize 541 -> 239 ns/op, relativePathInsideRoot 1778 -> 899, isPathInsideOrEqual 1076 -> 572, parseWslUncPath 57 -> 14. The loop-invariant root hoist added 176 ns/op on top of that (899 -> 723) at 7 hand-picked call sites, and cost a new exported factory whose input contract is the opposite of the one next to it, plus a function substitution in worktree/ownership.ts. Not worth 0.9 ms per storm. Prod diff: 2 files. New ratchet pins the single-factory surface.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { joinPath } from '@/lib/path'
|
||||
import { createFileExplorerWatchPathResolver } from '@/components/right-sidebar/useFileExplorerWatch'
|
||||
import { getExternalFileChangeRelativePath } from '@/components/right-sidebar/useFileExplorerWatch'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
import type { FsChangedPayload } from '../../../../shared/filesystem-entry-types'
|
||||
import {
|
||||
@@ -183,7 +183,6 @@ export function indexEditorExternalWatchBatchPaths(
|
||||
const deleteLookup = new IndexedPathLookup<IndexedPath>(allowAliases)
|
||||
const createOrUpdatePaths = new Map<string, string>()
|
||||
const changesByRelativePath = new Map<string, IndexedExternalWatchChange>()
|
||||
const watchPathResolver = createFileExplorerWatchPathResolver(scope.worktreePath)
|
||||
|
||||
for (const event of payload.events) {
|
||||
if (event.kind === 'overflow') {
|
||||
@@ -201,7 +200,8 @@ export function indexEditorExternalWatchBatchPaths(
|
||||
createOrUpdatePaths.set(eventPath.identity.normalizedPath, event.absolutePath)
|
||||
createOrUpdateLookup.add(eventPath, eventPath)
|
||||
}
|
||||
const relativePath = watchPathResolver.externalFileChangeRelativePath(
|
||||
const relativePath = getExternalFileChangeRelativePath(
|
||||
scope.worktreePath,
|
||||
event.absolutePath,
|
||||
event.isDirectory
|
||||
)
|
||||
|
||||
@@ -1,65 +1,42 @@
|
||||
import { joinPath, dirname, normalizeRelativePath } from '@/lib/path'
|
||||
import {
|
||||
createRelativePathInsideRootResolver,
|
||||
normalizeRuntimePathForComparison
|
||||
normalizeRuntimePathForComparison,
|
||||
relativePathInsideRoot
|
||||
} from '../../../../shared/cross-platform-path'
|
||||
|
||||
export function normalizeExplorerAbsolutePath(path: string): string {
|
||||
return path === '/' || /^[A-Za-z]:[\\/]$/.test(path) ? path : path.replace(/[\\/]+$/, '')
|
||||
}
|
||||
|
||||
export type FileExplorerWatchPathResolver = {
|
||||
canonicalize: (absolutePath: string) => string | null
|
||||
externalFileChangeRelativePath: (
|
||||
absolutePath: string,
|
||||
isDirectory: boolean | undefined
|
||||
) => string | null
|
||||
}
|
||||
|
||||
/** Folds the worktree root once per watcher batch rather than once per event in the storm. */
|
||||
export function createFileExplorerWatchPathResolver(
|
||||
worktreePath: string
|
||||
): FileExplorerWatchPathResolver {
|
||||
const insideRoot = createRelativePathInsideRootResolver(worktreePath)
|
||||
const rootPath = normalizeExplorerAbsolutePath(worktreePath)
|
||||
return {
|
||||
canonicalize: (absolutePath) => {
|
||||
const relativePath = insideRoot.resolve(absolutePath)
|
||||
if (relativePath === null) {
|
||||
return null
|
||||
}
|
||||
return relativePath === '' ? rootPath : joinPath(rootPath, relativePath)
|
||||
},
|
||||
externalFileChangeRelativePath: (absolutePath, isDirectory) => {
|
||||
if (isDirectory === true) {
|
||||
return null
|
||||
}
|
||||
const relativePath = insideRoot.resolve(absolutePath)
|
||||
if (relativePath === null || relativePath === '') {
|
||||
return null
|
||||
}
|
||||
// Why: EditorPanel reloads tabs only from a worktree-relative path, not the watcher's absolute one; normalize or contents go stale.
|
||||
return normalizeRelativePath(relativePath)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function getExternalFileChangeRelativePath(
|
||||
worktreePath: string,
|
||||
absolutePath: string,
|
||||
isDirectory: boolean | undefined
|
||||
): string | null {
|
||||
return createFileExplorerWatchPathResolver(worktreePath).externalFileChangeRelativePath(
|
||||
absolutePath,
|
||||
isDirectory
|
||||
)
|
||||
if (isDirectory === true) {
|
||||
return null
|
||||
}
|
||||
|
||||
const relativePath = relativePathInsideRoot(worktreePath, absolutePath)
|
||||
if (relativePath === null || relativePath === '') {
|
||||
return null
|
||||
}
|
||||
|
||||
// Why: EditorPanel reloads tabs only from a worktree-relative path, not the watcher's absolute one; normalize or contents go stale.
|
||||
return normalizeRelativePath(relativePath)
|
||||
}
|
||||
|
||||
export function canonicalizeFileExplorerWatchPath(
|
||||
worktreePath: string,
|
||||
absolutePath: string
|
||||
): string | null {
|
||||
return createFileExplorerWatchPathResolver(worktreePath).canonicalize(absolutePath)
|
||||
const relativePath = relativePathInsideRoot(worktreePath, absolutePath)
|
||||
if (relativePath === null) {
|
||||
return null
|
||||
}
|
||||
|
||||
const rootPath = normalizeExplorerAbsolutePath(worktreePath)
|
||||
return relativePath === '' ? rootPath : joinPath(rootPath, relativePath)
|
||||
}
|
||||
|
||||
export function createCachedDirPathIndex(
|
||||
|
||||
@@ -11,8 +11,8 @@ import {
|
||||
clearStalePendingReveal
|
||||
} from './file-explorer-watcher-reconcile'
|
||||
import {
|
||||
canonicalizeFileExplorerWatchPath,
|
||||
createCachedDirPathIndex,
|
||||
createFileExplorerWatchPathResolver,
|
||||
normalizeExplorerAbsolutePath,
|
||||
parentDirForWatchPath,
|
||||
resolveCachedDirPath
|
||||
@@ -71,7 +71,6 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA
|
||||
let cachedDirPathIndex: ReadonlyMap<string, string> | undefined
|
||||
const cachePathIndex = (): ReadonlyMap<string, string> =>
|
||||
(cachedDirPathIndex ??= createCachedDirPathIndex(cache))
|
||||
const watchPathResolver = createFileExplorerWatchPathResolver(currentWorktreePath)
|
||||
const cachedDirsToPurge = new Set<string>()
|
||||
const reconciledRenameSources = new Set<string>()
|
||||
let needsFullRefresh = false
|
||||
@@ -88,7 +87,7 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA
|
||||
break
|
||||
}
|
||||
|
||||
const normalizedPath = watchPathResolver.canonicalize(evt.absolutePath)
|
||||
const normalizedPath = canonicalizeFileExplorerWatchPath(currentWorktreePath, evt.absolutePath)
|
||||
if (!normalizedPath) {
|
||||
continue
|
||||
}
|
||||
@@ -140,7 +139,7 @@ export function processFileExplorerFsPayload(args: ProcessFileExplorerFsPayloadA
|
||||
}
|
||||
if (evt.kind === 'rename') {
|
||||
const oldPath = evt.oldAbsolutePath
|
||||
? watchPathResolver.canonicalize(evt.oldAbsolutePath)
|
||||
? canonicalizeFileExplorerWatchPath(currentWorktreePath, evt.oldAbsolutePath)
|
||||
: null
|
||||
const cachedOldDir = oldPath
|
||||
? resolveCachedDirPath(cache, oldPath, currentWorktreePath, cachePathIndex)
|
||||
|
||||
@@ -20,7 +20,6 @@ import { processFileExplorerFsPayload } from './file-explorer-watch-reconcile'
|
||||
|
||||
export {
|
||||
canonicalizeFileExplorerWatchPath,
|
||||
createFileExplorerWatchPathResolver,
|
||||
getExternalFileChangeRelativePath,
|
||||
resolveCachedDirPath
|
||||
} from './file-explorer-watch-path'
|
||||
|
||||
@@ -94,7 +94,7 @@ function generateRoot(random: () => number, candidate: string): string {
|
||||
|
||||
// ─── Differential fuzz ───────────────────────────────────────────────
|
||||
|
||||
const FUZZ_ITERATIONS = 200_000
|
||||
const FUZZ_ITERATIONS = 20_000
|
||||
|
||||
describe('guarded path normalization matches the pre-guard implementation', () => {
|
||||
it(`agrees on every export across ${FUZZ_ITERATIONS} seeded paths`, () => {
|
||||
@@ -192,17 +192,11 @@ describe('guarded path normalization matches the pre-guard implementation', () =
|
||||
) {
|
||||
record('createNormalizedPathInsideOrEqualMatcher', path, root)
|
||||
}
|
||||
const expectedRelative = unguarded.relativePathInsideRoot(root, path)
|
||||
if (guarded.relativePathInsideRoot(root, path) !== expectedRelative) {
|
||||
if (
|
||||
guarded.relativePathInsideRoot(root, path) !== unguarded.relativePathInsideRoot(root, path)
|
||||
) {
|
||||
record('relativePathInsideRoot', path, root)
|
||||
}
|
||||
const resolver = guarded.createRelativePathInsideRootResolver(root)
|
||||
if (resolver.resolve(path) !== expectedRelative) {
|
||||
record('createRelativePathInsideRootResolver.resolve', path, root)
|
||||
}
|
||||
if (resolver.comparisonRoot !== unguarded.normalizeRuntimePathForComparison(root)) {
|
||||
record('createRelativePathInsideRootResolver.comparisonRoot', path, root)
|
||||
}
|
||||
}
|
||||
|
||||
expect(mismatches).toEqual([])
|
||||
@@ -250,11 +244,9 @@ describe('guards still do the work when the fast path does not apply', () => {
|
||||
// ─── Regression guards: counted work, not wall clock ─────────────────
|
||||
|
||||
const originalReplace = String.prototype.replace
|
||||
const originalNormalize = String.prototype.normalize
|
||||
|
||||
afterEach(() => {
|
||||
String.prototype.replace = originalReplace
|
||||
String.prototype.normalize = originalNormalize
|
||||
})
|
||||
|
||||
function countReplaceCalls(run: () => void): number {
|
||||
@@ -271,20 +263,6 @@ function countReplaceCalls(run: () => void): number {
|
||||
return calls
|
||||
}
|
||||
|
||||
function countNormalizeCalls(run: () => void): number {
|
||||
let calls = 0
|
||||
String.prototype.normalize = function (this: string, ...args: never[]) {
|
||||
calls++
|
||||
return originalNormalize.apply(this, args as never)
|
||||
} as typeof String.prototype.normalize
|
||||
try {
|
||||
run()
|
||||
} finally {
|
||||
String.prototype.normalize = originalNormalize
|
||||
}
|
||||
return calls
|
||||
}
|
||||
|
||||
const CLEAN_POSIX_PATH =
|
||||
'/Users/nwparker/orca/workspaces/orca/perf/src/renderer/src/components/x.ts'
|
||||
|
||||
@@ -305,27 +283,34 @@ describe('no-op regex passes stay skipped', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('a fan-out folds its root once, not once per candidate', () => {
|
||||
const root = '/Users/nwparker/orca/workspaces/orca/perf'
|
||||
const candidates = Array.from({ length: 50 }, (_, index) => `${root}/src/file-${index}.ts`)
|
||||
// ─── One root-bound factory, one input contract ──────────────────────
|
||||
|
||||
it('normalizes the root a single time across the whole batch', () => {
|
||||
// 1 root fold + 2 per candidate (comparison key, then the NFC Windows-ness probe).
|
||||
const calls = countNormalizeCalls(() => {
|
||||
const resolver = guarded.createRelativePathInsideRootResolver(root)
|
||||
for (const candidate of candidates) {
|
||||
resolver.resolve(candidate)
|
||||
}
|
||||
})
|
||||
expect(calls).toBe(1 + candidates.length * 2)
|
||||
/**
|
||||
* `createNormalizedPathInsideOrEqualMatcher` demands an already-normalized candidate because
|
||||
* `normalizeRuntimePathForComparison` is not idempotent. A sibling factory on the same root that
|
||||
* took RAW candidates would put two opposite contracts one line apart, and mixing them up returns
|
||||
* "outside the root" rather than throwing. Hoisting a root out of a loop is worth ~0.2 us/event;
|
||||
* this is the price. Keep the raw-candidate entry point the plain `relativePathInsideRoot` call.
|
||||
*/
|
||||
describe('cross-platform-path exposes a single root-bound factory', () => {
|
||||
it('has no raw-candidate sibling to the normalized matcher', () => {
|
||||
expect(Object.keys(guarded).filter((name) => name.startsWith('create'))).toEqual([
|
||||
'createNormalizedPathInsideOrEqualMatcher'
|
||||
])
|
||||
})
|
||||
|
||||
it('costs strictly more when the root is re-folded per candidate', () => {
|
||||
const perCall = countNormalizeCalls(() => {
|
||||
for (const candidate of candidates) {
|
||||
guarded.relativePathInsideRoot(root, candidate)
|
||||
}
|
||||
})
|
||||
expect(perCall).toBe(candidates.length * 3)
|
||||
it('shows what mixing the two contracts would cost', () => {
|
||||
const root = '//wsl.localhost/Ubuntu/Repo'
|
||||
const candidate = '//wsl.localhost/Ubuntu/Repo/src/App.tsx'
|
||||
const normalizedCandidate = guarded.normalizeRuntimePathForComparison(candidate)
|
||||
expect(guarded.normalizeRuntimePathForComparison(normalizedCandidate)).not.toBe(
|
||||
normalizedCandidate
|
||||
)
|
||||
|
||||
const matcher = guarded.createNormalizedPathInsideOrEqualMatcher(root)
|
||||
expect(matcher(normalizedCandidate)).toBe(true)
|
||||
// The raw spelling a resolver would accept is silently reported as outside the root.
|
||||
expect(matcher(candidate)).toBe(false)
|
||||
expect(guarded.relativePathInsideRoot(root, candidate)).toBe('src/App.tsx')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -192,50 +192,28 @@ export function isPathInsideOrEqual(rootPath: string, candidatePath: string): bo
|
||||
)
|
||||
}
|
||||
|
||||
export type RelativePathInsideRootResolver = {
|
||||
/** The root's comparison key, so rankers can size it without a second normalize. */
|
||||
readonly comparisonRoot: string
|
||||
resolve: (candidatePath: string) => string | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-normalizes the root so a fan-out normalizes it once, not once per candidate.
|
||||
*
|
||||
* Unlike `createNormalizedPathInsideOrEqualMatcher`, candidates are passed raw: the returned
|
||||
* suffix has to be sliced out of the caller's own spelling, so the resolver needs both forms.
|
||||
*/
|
||||
export function createRelativePathInsideRootResolver(
|
||||
rootPath: string
|
||||
): RelativePathInsideRootResolver {
|
||||
export function relativePathInsideRoot(rootPath: string, candidatePath: string): string | null {
|
||||
// Why: decide Windows-ness on the same NFC form the comparison key uses, or the
|
||||
// two disagree (U+212A folds to 'K', making only one side a drive path) and the
|
||||
// segment counts desync. Only the branch test sees NFC — the sliced string stays
|
||||
// raw so the returned suffix remains byte-exact.
|
||||
const normalizedCandidate = trimRuntimePathTrailingSlash(
|
||||
isWindowsAbsolutePathLike(candidatePath.normalize('NFC'))
|
||||
? normalizeRuntimePathSeparators(candidatePath)
|
||||
: collapseRuntimePathSlashes(candidatePath)
|
||||
)
|
||||
const comparisonRoot = normalizeRuntimePathForComparison(rootPath)
|
||||
const comparisonCandidate = normalizeRuntimePathForComparison(candidatePath)
|
||||
|
||||
if (comparisonCandidate === comparisonRoot) {
|
||||
return ''
|
||||
}
|
||||
const isRoot = comparisonRoot === '/' || /^[a-z]:\/$/i.test(comparisonRoot)
|
||||
const comparisonPrefix = isRoot ? comparisonRoot : `${comparisonRoot}/`
|
||||
return {
|
||||
comparisonRoot,
|
||||
resolve: (candidatePath) => {
|
||||
const comparisonCandidate = normalizeRuntimePathForComparison(candidatePath)
|
||||
if (comparisonCandidate === comparisonRoot) {
|
||||
return ''
|
||||
}
|
||||
if (!comparisonCandidate.startsWith(comparisonPrefix)) {
|
||||
return null
|
||||
}
|
||||
// Why: decide Windows-ness on the same NFC form the comparison key uses, or the
|
||||
// two disagree (U+212A folds to 'K', making only one side a drive path) and the
|
||||
// segment counts desync. Only the branch test sees NFC — the sliced string stays
|
||||
// raw so the returned suffix remains byte-exact.
|
||||
const normalizedCandidate = trimRuntimePathTrailingSlash(
|
||||
isWindowsAbsolutePathLike(candidatePath.normalize('NFC'))
|
||||
? normalizeRuntimePathSeparators(candidatePath)
|
||||
: collapseRuntimePathSlashes(candidatePath)
|
||||
)
|
||||
return sliceCandidatePastRootSegments(comparisonRoot, normalizedCandidate)
|
||||
}
|
||||
if (!comparisonCandidate.startsWith(comparisonPrefix)) {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
export function relativePathInsideRoot(rootPath: string, candidatePath: string): string | null {
|
||||
return createRelativePathInsideRootResolver(rootPath).resolve(candidatePath)
|
||||
return sliceCandidatePastRootSegments(comparisonRoot, normalizedCandidate)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
* Centralized to stop local/relay listFiles from drifting on blocklist, ignores, exclusions,
|
||||
* timeouts, and buffering. See docs/design/share-quick-open-file-listing.md.
|
||||
*/
|
||||
import { createRelativePathInsideRootResolver } from './cross-platform-path'
|
||||
import { relativePathInsideRoot } from './cross-platform-path'
|
||||
|
||||
// ─── Hidden-dir blocklist ────────────────────────────────────────────
|
||||
|
||||
@@ -85,12 +85,11 @@ export function buildExcludePathPrefixes(rootPath: string, excludePaths?: unknow
|
||||
return []
|
||||
}
|
||||
const out: string[] = []
|
||||
const insideRoot = createRelativePathInsideRootResolver(rootPath)
|
||||
for (const raw of excludePaths) {
|
||||
if (typeof raw !== 'string' || raw.length === 0) {
|
||||
continue
|
||||
}
|
||||
const relativePath = insideRoot.resolve(raw)
|
||||
const relativePath = relativePathInsideRoot(rootPath, raw)
|
||||
if (relativePath === null) {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { ExecutionHostId } from './execution-host'
|
||||
import { createRelativePathInsideRootResolver } from './cross-platform-path'
|
||||
import { normalizeRuntimePathForComparison, relativePathInsideRoot } from './cross-platform-path'
|
||||
|
||||
export type RuntimeWorkspaceFileRoot = {
|
||||
workspaceId: string
|
||||
@@ -23,12 +23,11 @@ export function findRuntimeWorkspaceFileOwner(
|
||||
if (root.executionHostId !== executionHostId) {
|
||||
continue
|
||||
}
|
||||
const insideRoot = createRelativePathInsideRootResolver(root.rootPath)
|
||||
const relativePath = insideRoot.resolve(absolutePath)
|
||||
const relativePath = relativePathInsideRoot(root.rootPath, absolutePath)
|
||||
if (relativePath === null) {
|
||||
continue
|
||||
}
|
||||
const rootLength = insideRoot.comparisonRoot.length
|
||||
const rootLength = normalizeRuntimePathForComparison(root.rootPath).length
|
||||
if (
|
||||
rootLength > bestRootLength ||
|
||||
(rootLength === bestRootLength &&
|
||||
|
||||
@@ -1,8 +1,4 @@
|
||||
import {
|
||||
createNormalizedPathInsideOrEqualMatcher,
|
||||
normalizeRuntimePathForComparison,
|
||||
relativePathInsideRoot
|
||||
} from '../cross-platform-path'
|
||||
import { normalizeRuntimePathForComparison, relativePathInsideRoot } from '../cross-platform-path'
|
||||
import { parseWslUncPath } from '../wsl-paths'
|
||||
import {
|
||||
isRuntimePathAbsoluteForRepo,
|
||||
@@ -245,10 +241,9 @@ function isUnderFlatOrUntrustedOrcaRoot(
|
||||
worktreePath: string,
|
||||
knownOrcaLayouts: OrcaWorkspaceLayout[]
|
||||
): boolean {
|
||||
// Only containment is asked here, so fold the candidate once instead of once per layout.
|
||||
const comparisonWorktreePath = normalizeRuntimePathForComparison(worktreePath)
|
||||
for (const layout of knownOrcaLayouts) {
|
||||
if (!createNormalizedPathInsideOrEqualMatcher(layout.path)(comparisonWorktreePath)) {
|
||||
const relative = relativePathInsideRoot(layout.path, worktreePath)
|
||||
if (relative === null) {
|
||||
continue
|
||||
}
|
||||
if (!layout.nestWorkspaces) {
|
||||
@@ -265,9 +260,9 @@ function canClassifyAsExternal(
|
||||
if (knownOrcaLayouts.length === 0) {
|
||||
return false
|
||||
}
|
||||
const comparisonWorktreePath = normalizeRuntimePathForComparison(worktreePath)
|
||||
for (const layout of knownOrcaLayouts) {
|
||||
if (!createNormalizedPathInsideOrEqualMatcher(layout.path)(comparisonWorktreePath)) {
|
||||
const relative = relativePathInsideRoot(layout.path, worktreePath)
|
||||
if (relative === null) {
|
||||
continue
|
||||
}
|
||||
return layout.nestWorkspaces
|
||||
|
||||
Reference in New Issue
Block a user