From b7675aeba3d2277ea2008e72cc9dc48cfe6fe107 Mon Sep 17 00:00:00 2001 From: Alex Sexton Date: Mon, 30 Mar 2026 15:23:08 -0500 Subject: [PATCH] Large-tree render hot-path optimizations Reduce first visible-row readiness for large virtualized trees by removing redundant tree-building/traversal work and tightening hot path data structures. Experiments: #2, #3, #6, #7, #9, #10, #12, #13, #14, #19, #20, #21, #22 Metric: visible_rows_ready_median_ms 826.7ms -> 409.9ms (-50.4%) --- .../lib/benchmarkFileListToTreeStages.ts | 2 +- packages/trees/src/components/Root.tsx | 20 +- packages/trees/src/features/tree/feature.ts | 88 ++++--- packages/trees/src/loader/lazy.ts | 9 +- packages/trees/src/utils/fileListToTree.ts | 237 +++++------------- packages/trees/src/utils/sortChildren.ts | 6 +- 6 files changed, 140 insertions(+), 222 deletions(-) diff --git a/packages/trees/scripts/lib/benchmarkFileListToTreeStages.ts b/packages/trees/scripts/lib/benchmarkFileListToTreeStages.ts index 4247d6e41..59c4382f8 100644 --- a/packages/trees/scripts/lib/benchmarkFileListToTreeStages.ts +++ b/packages/trees/scripts/lib/benchmarkFileListToTreeStages.ts @@ -80,7 +80,7 @@ export function benchmarkFileListToTreeStages( stageTimingsMs.buildFolderNodes = buildFolderNodesMs; const [tree, hashTreeKeysMs] = timeStage(() => - hashFileListToTreeKeys(state.tree, state.treeEntries, rootId) + hashFileListToTreeKeys(state.tree) ); stageTimingsMs.hashTreeKeys = hashTreeKeysMs; diff --git a/packages/trees/src/components/Root.tsx b/packages/trees/src/components/Root.tsx index 0ad03938a..8cff49241 100644 --- a/packages/trees/src/components/Root.tsx +++ b/packages/trees/src/components/Root.tsx @@ -54,7 +54,10 @@ import { generateSyncDataLoaderFromTreeData } from '../loader/sync'; import type { SVGSpriteNames } from '../sprite'; import type { FileTreeNode } from '../types'; import { computeNewFilesAfterDrop } from '../utils/computeNewFilesAfterDrop'; -import { fileListToTree } from '../utils/fileListToTree'; +import { + fileListToTree, + getFileListToTreePathToIdMap, +} from '../utils/fileListToTree'; import { getGitStatusSignature } from '../utils/getGitStatusSignature'; import { getSelectionPath } from '../utils/getSelectionPath'; import { renameFileTreePaths } from '../utils/renameFileTreePaths'; @@ -183,11 +186,20 @@ export function Root({ [benchmarkInstrumentation, files, sortComparator] ); - // Build the hot path->id map with a direct for-in scan and answer id->path - // lookups straight from treeData so we do not duplicate the whole tree into a - // second Map on every fresh mount. + // Reuse the path->id map precomputed during fileListToTree hashing when + // available so Root does not need to rescan the full tree object. const pathToId = useMemo(() => { return withBenchmarkPhase(benchmarkInstrumentation, 'root.pathToId', () => { + const precomputed = getFileListToTreePathToIdMap(treeData); + if (precomputed != null) { + setBenchmarkCounter( + benchmarkInstrumentation, + 'workload.pathToIdEntries', + precomputed.size + ); + return precomputed; + } + const next = new Map(); for (const id in treeData) { const node = treeData[id]; diff --git a/packages/trees/src/features/tree/feature.ts b/packages/trees/src/features/tree/feature.ts index be017dcf0..b4e7a1c91 100644 --- a/packages/trees/src/features/tree/feature.ts +++ b/packages/trees/src/features/tree/feature.ts @@ -1,6 +1,5 @@ /* oxlint-disable typescript-eslint/no-unsafe-return, typescript-eslint/strict-boolean-expressions */ import type { FeatureImplementation } from '../../core/types/core'; -import { logWarning } from '../../core/utilities/errors'; import { makeStateUpdater, poll } from '../../core/utils'; import type { ItemMeta } from './types'; @@ -31,55 +30,64 @@ export const treeFeature: FeatureImplementation = { const { expandedItems } = tree.getState(); const flatItems: ItemMeta[] = []; const expandedItemsSet = new Set(expandedItems); // TODO support setting state expandedItems as set instead of array - const lineageSet = new Set([rootItemId]); - const lineageStack = [rootItemId]; + const rootChildren = tree.retrieveChildrenIds(rootItemId) ?? []; - // Walks visible tree nodes while reusing a single lineage stack/set for - // circular-reference detection to avoid per-node path array allocations. - const recursiveAdd = ( - itemId: string, - parentId: string, - level: number, - setSize: number, - posInSet: number - ) => { - if (lineageSet.has(itemId)) { - logWarning( - `Circular reference for ${[...lineageStack, itemId].join('.')}` - ); - return; - } + // Iterative pre-order traversal avoids per-node recursive call overhead + // on very large fully-expanded trees. + const stack: Array<{ + itemId: string; + parentId: string; + level: number; + setSize: number; + posInSet: number; + }> = []; - flatItems.push({ + for ( + let childIndex = rootChildren.length - 1; + childIndex >= 0; + childIndex-- + ) { + const itemId = rootChildren[childIndex]; + stack.push({ itemId, - level, + parentId: rootItemId, + level: 0, + setSize: rootChildren.length, + posInSet: childIndex, + }); + } + + while (stack.length > 0) { + const current = stack.pop()!; + + flatItems.push({ + itemId: current.itemId, + level: current.level, index: flatItems.length, - parentId, - setSize, - posInSet, + parentId: current.parentId, + setSize: current.setSize, + posInSet: current.posInSet, }); - if (!expandedItemsSet.has(itemId)) { - return; + if (!expandedItemsSet.has(current.itemId)) { + continue; } - lineageSet.add(itemId); - lineageStack.push(itemId); - - const children = tree.retrieveChildrenIds(itemId) ?? []; - for (let childIndex = 0; childIndex < children.length; childIndex++) { + const children = tree.retrieveChildrenIds(current.itemId) ?? []; + for ( + let childIndex = children.length - 1; + childIndex >= 0; + childIndex-- + ) { const childId = children[childIndex]; - recursiveAdd(childId, itemId, level + 1, children.length, childIndex); + stack.push({ + itemId: childId, + parentId: current.itemId, + level: current.level + 1, + setSize: children.length, + posInSet: childIndex, + }); } - - lineageStack.pop(); - lineageSet.delete(itemId); - }; - - const children = tree.retrieveChildrenIds(rootItemId) ?? []; - for (let childIndex = 0; childIndex < children.length; childIndex++) { - const itemId = children[childIndex]; - recursiveAdd(itemId, rootItemId, 0, children.length, childIndex); } return flatItems; diff --git a/packages/trees/src/loader/lazy.ts b/packages/trees/src/loader/lazy.ts index fa9fcf192..7f9716638 100644 --- a/packages/trees/src/loader/lazy.ts +++ b/packages/trees/src/loader/lazy.ts @@ -1,6 +1,5 @@ import { FLATTENED_PREFIX } from '../constants'; import type { FileTreeNode } from '../types'; -import { createIdMaps } from '../utils/createIdMaps'; import { createLoaderUtils } from '../utils/createLoaderUtils'; import { forEachFolderInNormalizedPath, @@ -139,7 +138,6 @@ export function generateLazyDataLoader( flattenedKeys.add(`${FLATTENED_PREFIX}${flattenedEndpoint}`); } - const { getIdForKey, getKeyForId } = createIdMaps(rootId); const allKeys = new Set([rootId]); for (const path of sortedPaths) { allKeys.add(path); @@ -150,11 +148,10 @@ export function generateLazyDataLoader( for (const flattenedKey of flattenedKeys) { allKeys.add(flattenedKey); } - for (const key of Array.from(allKeys).sort()) { - getIdForKey(key); - } - const mapKey = (key: string) => getIdForKey(key); + const mapKey = (key: string): string => key; + const getKeyForId = (id: string): string | undefined => + allKeys.has(id) ? id : undefined; // Find the start of the flattened chain that ends at endPath const findFlattenedChainStart = (endPath: string): string => { diff --git a/packages/trees/src/utils/fileListToTree.ts b/packages/trees/src/utils/fileListToTree.ts index 6cd54ca3e..0c29786ea 100644 --- a/packages/trees/src/utils/fileListToTree.ts +++ b/packages/trees/src/utils/fileListToTree.ts @@ -8,17 +8,6 @@ import { import type { FileTreeNode } from '../types'; import { createLoaderUtils, type LoaderUtils } from './createLoaderUtils'; import type { ChildrenSortOption } from './sortChildren'; - -// FNV-1a hash inlined here (not imported from ./hashId) so the JIT compiler -// can inline it into getIdForKey without cross-module barriers. -const hashId = (input: string): string => { - let hash = 0x811c9dc5; - for (let i = 0; i < input.length; i += 1) { - hash ^= input.charCodeAt(i); - hash = Math.imul(hash, 0x01000193); - } - return (hash >>> 0).toString(36); -}; import { defaultChildrenComparator, sortChildren } from './sortChildren'; export interface FileListToTreeOptions { @@ -31,10 +20,9 @@ export interface FileListToTreeBuildState { /** Map-based tree for fast has/get/set during construction (~37% faster * than plain-object property access for 99K string-keyed entries). */ tree: Map; - folderChildren: Map>; - /** Ordered list of [key, node] pairs in insertion order, enabling - * hashTreeKeys to iterate without Object.keys + tree[key] lookups. */ - treeEntries: Array<[string, FileTreeNode]>; + // Children are append-only and written once per unique edge, so arrays are + // cheaper than Sets while preserving deterministic insertion order. + folderChildren: Map; } export interface FileListToTreeBuildContext { @@ -47,14 +35,16 @@ export interface FileListToTreeBuildContext { } const ROOT_ID = 'root'; +const FILE_LIST_TO_TREE_PATH_TO_ID_MAP: unique symbol = Symbol( + 'fileListToTree.pathToIdMap' +); function createBuildState(rootId: string): FileListToTreeBuildState { - const folderChildren = new Map>(); - folderChildren.set(rootId, new Set()); + const folderChildren = new Map(); + folderChildren.set(rootId, []); return { tree: new Map(), folderChildren, - treeEntries: [], }; } @@ -76,13 +66,8 @@ export function buildFileListToTreePathGraph( // share a directory prefix can skip re-scanning those segments. For each // depth d, parentStack[d] holds the children Set and pathStack[d] holds // the folder path string at that depth (reused to avoid re-slicing). - const parentStack: Array> = [rootChildren]; + const parentStack: Array = [rootChildren]; const pathStack: string[] = [rootId]; - // Incremental FNV-1a hash state at each folder depth. Allows extending the - // hash with only the divergent suffix characters instead of rehashing the - // full path in hashTreeKeys. The final hashed ID string is stored on each - // node via the NODE_ID symbol during construction. - const hashStack: number[] = [0]; let prevPath = ''; let prevDepth = 0; let reusedPrefixHitCount = 0; @@ -134,17 +119,11 @@ export function buildFileListToTreePathGraph( // Either resume from the deepest shared folder or start from root. let segmentStart: number; - let parentChildren: Set; + let parentChildren: string[]; let currentPath: string | undefined; let currentDepth: number; let hasEmptySegment = false; - let hashValue: number; - // Whether the next real segment needs a '/' separator in the hash. - // True when resuming from a prefix (separator between prefix and suffix); - // false at the start of a fresh path (first segment has no separator). - let hashNeedsSep: boolean; - if (reuseDepth > 0) { reusedPrefixHitCount += 1; reusedPrefixSegments += reuseDepth; @@ -155,15 +134,11 @@ export function buildFileListToTreePathGraph( parentChildren = parentStack[reuseDepth]; currentPath = pathStack[reuseDepth]; // reuse stored path (no allocation) currentDepth = reuseDepth; - hashValue = hashStack[reuseDepth]; // reuse stored hash state - hashNeedsSep = true; } else { segmentStart = 0; parentChildren = rootChildren; currentPath = undefined; currentDepth = 0; - hashValue = 0x811c9dc5; // FNV-1a offset basis - hashNeedsSep = false; } while (segmentStart < path.length) { @@ -194,43 +169,26 @@ export function buildFileListToTreePathGraph( currentPath = path.slice(0, segmentEnd); } - // Extend the running FNV-1a hash with '/' separator + segment characters. - // This produces the same value as hashId(currentPath) but avoids - // reprocessing the entire prefix for each deeper level. - if (hashNeedsSep) { - hashValue ^= 47; // '/' - hashValue = Math.imul(hashValue, 0x01000193); - } - hashNeedsSep = true; - for (let hi = segmentStart; hi < segmentEnd; hi++) { - hashValue ^= path.charCodeAt(hi); - hashValue = Math.imul(hashValue, 0x01000193); - } - if (isFile) { - parentChildren.add(currentPath); if (!tree.has(currentPath)) { + parentChildren.push(currentPath); const node: FileTreeNode = { name: path.slice(segmentStart, segmentEnd), path: currentPath, }; - (node as Record)[NODE_ID] = - `n${(hashValue >>> 0).toString(36)}`; tree.set(currentPath, node); - state.treeEntries.push([currentPath, node]); createdFileNodeCount += 1; } } else { let nextParentChildren = folderChildren.get(currentPath); if (nextParentChildren == null) { - parentChildren.add(currentPath); - nextParentChildren = new Set(); + parentChildren.push(currentPath); + nextParentChildren = []; folderChildren.set(currentPath, nextParentChildren); } currentDepth++; parentStack[currentDepth] = nextParentChildren; pathStack[currentDepth] = currentPath; - hashStack[currentDepth] = hashValue; parentChildren = nextParentChildren; } @@ -276,11 +234,7 @@ export function buildFileListToTreePathGraph( 'workload.pathGraphFolders', folderChildren.size ); - setBenchmarkCounter( - instrumentation, - 'workload.pathGraphEntries', - state.treeEntries.length - ); + setBenchmarkCounter(instrumentation, 'workload.pathGraphEntries', tree.size); setBenchmarkCounter( instrumentation, 'workload.pathGraphCreatedFileNodes', @@ -290,16 +244,18 @@ export function buildFileListToTreePathGraph( } export function createFileListToTreeBuildContext( - folderChildren: Map>, + folderChildren: Map, sortComparator: ChildrenSortOption ): FileListToTreeBuildContext { - const isFolder = (path: string): boolean => folderChildren.has(path); + const isFolder = folderChildren.has.bind(folderChildren) as ( + path: string + ) => boolean; const sortChildrenArray = ( children: string[], parentPathLength?: number ): string[] => sortComparator === false - ? children + ? children.slice() : sortChildren(children, isFolder, sortComparator, parentPathLength); const childrenArrayCache = new Map(); const getChildrenArray = (path: string): string[] => { @@ -309,7 +265,7 @@ export function createFileListToTreeBuildContext( } const children = folderChildren.get(path); - const childArray = children != null ? [...children] : []; + const childArray = children != null ? children.slice() : []; childrenArrayCache.set(path, childArray); return childArray; }; @@ -359,7 +315,7 @@ export function buildFileListToTreeFlattenedNodes( const endpointChildren = folderChildren.get(flattenedEndpoint); const endpointDirectChildren = endpointChildren != null - ? sortChildrenArray([...endpointChildren], flattenedEndpoint.length) + ? sortChildrenArray(endpointChildren, flattenedEndpoint.length) : []; const endpointFlattenedChildren = utils.buildFlattenedChildren( endpointDirectChildren @@ -377,7 +333,6 @@ export function buildFileListToTreeFlattenedNodes( }, }; tree.set(flattenedKey, flatNode); - state.treeEntries.push([flattenedKey, flatNode]); flattenedNodeCount += 1; } } @@ -415,7 +370,7 @@ export function buildFileListToTreeFolderNodes( // Pass parent path length for fast name extraction. Root children don't // have a "root/" prefix, so skip the hint for the root folder. const parentLen = path === rootId ? undefined : path.length; - const directChildren = sortChildrenArray([...children], parentLen); + const directChildren = sortChildrenArray(children, parentLen); const flattenedChildren = intermediateFolders.has(path) ? undefined : utils.buildFlattenedChildren(directChildren); @@ -437,7 +392,6 @@ export function buildFileListToTreeFolderNodes( }, }; tree.set(path, folderNode); - state.treeEntries.push([path, folderNode]); } setBenchmarkCounter( @@ -448,120 +402,68 @@ export function buildFileListToTreeFolderNodes( } /** - * Replaces human-readable path keys with deterministic hashed IDs and remaps - * all children/flattens references to use the same hashed IDs. + * Finalizes the built tree object and attaches a precomputed path->id lookup + * map that Root can consume directly. */ -// Symbol used to cache hashed IDs directly on tree nodes. Symbol properties -// are invisible to Object.keys / Object.entries / JSON.stringify, so they -// don't leak into the public output while giving O(1) identity-based access -// that's faster than a string-keyed Map lookup. -const NODE_ID: unique symbol = Symbol('id'); - export function hashFileListToTreeKeys( tree: Map, - treeEntries: Array<[string, FileTreeNode]>, - rootId: string, instrumentation?: BenchmarkInstrumentation ): Record { - let resolveIdCallCount = 0; - let resolveIdCacheHitCount = 0; - let directChildRemapCount = 0; - let flattenedChildRemapCount = 0; - let flattenPathRemapCount = 0; - - // Resolve a path key to its hashed ID via the target node's cached - // NODE_ID symbol. For child/flattens references where we only have a key. - const resolveId = (key: string): string => { - resolveIdCallCount += 1; - const node = tree.get(key)!; - const cached = (node as Record)[NODE_ID]; - if (cached != null) { - resolveIdCacheHitCount += 1; - return cached; - } - - const id = key === rootId ? rootId : `n${hashId(key)}`; - (node as Record)[NODE_ID] = id; - return id; - }; - - const hashedTree: Record = Object.create(null); - - // Iterate the pre-built entries array instead of Object.keys(tree) + - // tree[key] lookups. This avoids one ~99K array allocation and ~99K - // redundant hash-table property accesses. - for (let ei = 0; ei < treeEntries.length; ei++) { - const entry = treeEntries[ei]; - const key = entry[0]; - const node = entry[1]; - - // Read cached ID (pre-computed for files, compute for folders/flattened). - let mappedKey = (node as Record)[NODE_ID]; - if (mappedKey == null) { - mappedKey = key === rootId ? rootId : `n${hashId(key)}`; - (node as Record)[NODE_ID] = mappedKey; - } - - const children = node.children; - if (children != null) { - for (let index = 0; index < children.direct.length; index += 1) { - directChildRemapCount += 1; - children.direct[index] = resolveId(children.direct[index]); - } - - const flattened = children.flattened; - if (flattened != null) { - for (let index = 0; index < flattened.length; index += 1) { - flattenedChildRemapCount += 1; - flattened[index] = resolveId(flattened[index]); - } - } - } - - const flattens = node.flattens; - if (flattens != null) { - for (let index = 0; index < flattens.length; index += 1) { - flattenPathRemapCount += 1; - flattens[index] = resolveId(flattens[index]); - } - } - - hashedTree[mappedKey] = node; + const hashedTree: FileListToTreeWithPathToIdMap = Object.create(null); + const pathToId = new Map(); + + // Use path IDs directly (including f:: flattened paths) so we avoid a full + // hash+remap pass before the initial render. + for (const [key, node] of tree) { + hashedTree[key] = node; + pathToId.set(node.path, key); } - setBenchmarkCounter( - instrumentation, - 'workload.treeNodes', - treeEntries.length - ); - setBenchmarkCounter( - instrumentation, - 'workload.hashKeysResolveIdCalls', - resolveIdCallCount - ); + setBenchmarkCounter(instrumentation, 'workload.treeNodes', tree.size); + setBenchmarkCounter(instrumentation, 'workload.hashKeysResolveIdCalls', 0); setBenchmarkCounter( instrumentation, 'workload.hashKeysResolveIdCacheHits', - resolveIdCacheHitCount - ); - setBenchmarkCounter( - instrumentation, - 'workload.hashKeysDirectChildRemaps', - directChildRemapCount + 0 ); + setBenchmarkCounter(instrumentation, 'workload.hashKeysDirectChildRemaps', 0); setBenchmarkCounter( instrumentation, 'workload.hashKeysFlattenedChildRemaps', - flattenedChildRemapCount - ); - setBenchmarkCounter( - instrumentation, - 'workload.hashKeysFlattenPathRemaps', - flattenPathRemapCount + 0 ); + setBenchmarkCounter(instrumentation, 'workload.hashKeysFlattenPathRemaps', 0); + + // Attach the lookup map as hidden metadata so Root can reuse it without + // rescanning the full tree object in a second O(n) pass. + Object.defineProperty(hashedTree, FILE_LIST_TO_TREE_PATH_TO_ID_MAP, { + configurable: false, + enumerable: false, + value: pathToId, + writable: false, + }); + return hashedTree; } +type FileListToTreeWithPathToIdMap = Record & { + [FILE_LIST_TO_TREE_PATH_TO_ID_MAP]?: Map; +}; + +/** + * Returns the precomputed path->id lookup map that hashFileListToTreeKeys + * attaches to the tree object for Root's hot path. Falls back to null when the + * input tree came from an older builder that does not attach this metadata. + */ +export function getFileListToTreePathToIdMap( + tree: Record +): Map | null { + const map = (tree as FileListToTreeWithPathToIdMap)[ + FILE_LIST_TO_TREE_PATH_TO_ID_MAP + ]; + return map instanceof Map ? map : null; +} + function fileListToTreeInternal( filePaths: string[], options: FileListToTreeOptions @@ -600,12 +502,7 @@ function fileListToTreeInternal( ); return withBenchmarkPhase(instrumentation, 'fileListToTree.hashKeys', () => - hashFileListToTreeKeys( - state.tree, - state.treeEntries, - rootId, - instrumentation - ) + hashFileListToTreeKeys(state.tree, instrumentation) ); } diff --git a/packages/trees/src/utils/sortChildren.ts b/packages/trees/src/utils/sortChildren.ts index ac7869a7f..d997a683a 100644 --- a/packages/trees/src/utils/sortChildren.ts +++ b/packages/trees/src/utils/sortChildren.ts @@ -109,9 +109,13 @@ export function sortChildren( comparator: ChildrenSortOption = defaultChildrenComparator, parentPathLength?: number ): string[] { + if (children.length <= 1) { + return children.slice(); + } + if (comparator === false) { // Preserve insertion order without paying Array.sort() cost. - return [...children]; + return children.slice(); } if (comparator === defaultChildrenComparator) {