From 7b911f452c3879995fcfd63500b3a80d19e1ae15 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 8 May 2024 14:59:10 +0200 Subject: [PATCH] perf: improve performance of peer dependencies resolution --- pkg-manager/resolve-dependencies/src/index.ts | 2 + .../src/resolveDependencies.ts | 7 + .../src/resolveDependencyTree.ts | 3 + .../resolve-dependencies/src/resolvePeers.ts | 386 +++++++++++------- 4 files changed, 250 insertions(+), 148 deletions(-) diff --git a/pkg-manager/resolve-dependencies/src/index.ts b/pkg-manager/resolve-dependencies/src/index.ts index 7bb96a559f5..1cc6364b143 100644 --- a/pkg-manager/resolve-dependencies/src/index.ts +++ b/pkg-manager/resolve-dependencies/src/index.ts @@ -133,6 +133,7 @@ export async function resolveDependencies ( wantedToBeSkippedPackageIds, appliedPatches, time, + allPeerDeps, } = await resolveDependencyTree(projectsToResolve, opts) // We only check whether patches were applied in cases when the whole lockfile was reanalyzed. @@ -191,6 +192,7 @@ export async function resolveDependencies ( dependenciesByProjectId, peerDependencyIssuesByProjects, } = await resolvePeers({ + allPeers: allPeerDeps, dependenciesTree, dedupePeerDependents: opts.dedupePeerDependents, dedupeInjectedDeps: opts.dedupeInjectedDeps, diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index 216837b45a7..51dae704370 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -130,6 +130,7 @@ export interface ChildrenByParentDepPath { } export interface ResolutionContext { + allPeerDeps: Set autoInstallPeers: boolean autoInstallPeersFromHighestMatch: boolean allowBuild?: (pkgName: string) => boolean @@ -1329,6 +1330,12 @@ async function resolveDependency ( ctx.outdatedDependencies[pkgResponse.body.id] = pkgResponse.body.latest } + Object.keys(pkg.peerDependencies ?? {}).forEach((name) => { + ctx.allPeerDeps.add(name) + }) + Object.keys(pkg.peerDependenciesMeta ?? {}).forEach((name) => { + ctx.allPeerDeps.add(name) + }) // In case of leaf dependencies (dependencies that have no prod deps or peer deps), // we only ever need to analyze one leaf dep in a graph, so the nodeId can be short and stateless. const nodeId = pkgIsLeaf(pkg) diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts index bf58642f9ae..6987602fa09 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts @@ -107,6 +107,7 @@ export interface ResolveDependenciesOptions { } export interface ResolveDependencyTreeResult { + allPeerDeps: Set dependenciesTree: DependenciesTree outdatedDependencies: { [pkgId: string]: string @@ -160,6 +161,7 @@ export async function resolveDependencyTree ( workspacePackages: opts.workspacePackages, missingPeersOfChildrenByPkgId: {}, hoistPeers: autoInstallPeers || opts.dedupePeerDependents, + allPeerDeps: new Set(), } const resolveArgs: ImporterToResolve[] = importers.map((importer) => { @@ -252,6 +254,7 @@ export async function resolveDependencyTree ( wantedToBeSkippedPackageIds, appliedPatches: ctx.appliedPatches, time, + allPeerDeps: ctx.allPeerDeps, } } diff --git a/pkg-manager/resolve-dependencies/src/resolvePeers.ts b/pkg-manager/resolve-dependencies/src/resolvePeers.ts index 32b38f4396c..416dfb75d47 100644 --- a/pkg-manager/resolve-dependencies/src/resolvePeers.ts +++ b/pkg-manager/resolve-dependencies/src/resolvePeers.ts @@ -66,6 +66,7 @@ export type DependenciesByProjectId = Record> export async function resolvePeers ( opts: { + allPeers: Set projects: ProjectToResolve[] dependenciesTree: DependenciesTree virtualStoreDir: string @@ -104,6 +105,8 @@ export async function resolvePeers ( // eslint-disable-next-line no-await-in-loop const { finishing } = await resolvePeersOfChildren(directNodeIdsByAlias, pkgsByName, { + allPeers: opts.allPeers, + getParentPkgs: {}, dependenciesTree: opts.dependenciesTree, depGraph, lockfileDir: opts.lockfileDir, @@ -286,7 +289,7 @@ function createPkgsByName ( } interface PeersCacheItem { - depPath: pDefer.DeferredPromise + depPath: pDefer.DeferredPromise & { value?: string } resolvedPeers: Map missingPeers: Set } @@ -307,10 +310,16 @@ interface ResolvePeersContext { type CalculateDepPath = (cycles: string[][]) => Promise type FinishingResolutionPromise = Promise -async function resolvePeersOfNode ( +type ResolvePeersOfNodeResult = PeersResolution & { finishing?: FinishingResolutionPromise, calculateDepPath?: CalculateDepPath } + +type GetParentPkgs = () => Record + +function resolvePeersOfNode ( nodeId: string, parentParentPkgs: ParentRefs, ctx: ResolvePeersContext & { + allPeers: Set + getParentPkgs: Record dependenciesTree: DependenciesTree depGraph: GenericDependenciesGraph virtualStoreDir: string @@ -321,7 +330,7 @@ async function resolvePeersOfNode ( rootDir: string lockfileDir: string } -): Promise { +): ResolvePeersOfNodeResult | (() => Promise) { const node = ctx.dependenciesTree.get(nodeId)! if (node.depth === -1) return { resolvedPeers: new Map(), missingPeers: new Set() } const resolvedPackage = node.resolvedPackage as T @@ -369,12 +378,33 @@ async function resolvePeersOfNode ( return false } const parentDepPath = (ctx.dependenciesTree.get(parentPkgNodeId)!.resolvedPackage as T).depPath + // TODO: this all should be done recursively for each parent if (!ctx.purePkgs.has(parentDepPath)) { - if ( - newPendingCacheSearches.includes(parentDepPath) || - findHit(parentDepPath, newPendingCacheSearches) - ) continue - return false + const cachedDepPath = (ctx.dependenciesTree.get(cachedNodeId)!.resolvedPackage as T).depPath + if (parentDepPath !== cachedDepPath) return false + // console.log(ctx.getParentPkgs) + const cachedParentPkgs = ctx.getParentPkgs[cachedNodeId]?.() + if (!cachedParentPkgs) return false + const thisParentParentPkgs = ctx.getParentPkgs[parentPkgNodeId]?.() + if (!thisParentParentPkgs) return false + // console.log('cached', cachedParentPkgs) + // console.log('this', thisParentParentPkgs) + if (Object.keys(cachedParentPkgs).length !== Object.keys(thisParentParentPkgs).length || !Object.entries(cachedParentPkgs).every(([name, depPathOrVersion]) => { + return depPathOrVersion === thisParentParentPkgs[name] + // const currentParent = parentPkgs[name] + // if (!currentParent.nodeId) { + // return currentParent.version === depPathOrVersion + // } + // const currentParentDepPath = (ctx.dependenciesTree.get(currentParent.nodeId)!.resolvedPackage as T).depPath + // return currentParentDepPath === depPathOrVersion + })) return false + // console.log('GOOOD') + continue + // if ( + // newPendingCacheSearches.includes(parentDepPath) || + // findHit(parentDepPath, newPendingCacheSearches) + // ) continue + // return false } const cachedDepPath = (ctx.dependenciesTree.get(cachedNodeId)!.resolvedPackage as T).depPath if (parentDepPath !== cachedDepPath) return false @@ -386,6 +416,17 @@ async function resolvePeersOfNode ( }) } if (hit != null) { + if (hit.depPath.value) { + const depPath = hit.depPath.value + ctx.pathsByNodeId.set(nodeId, depPath) + ctx.depGraph[depPath].depth = Math.min(ctx.depGraph[depPath].depth, node.depth) + ctx.pathsByNodeIdPromises.get(nodeId)!.resolve(depPath) + return { + missingPeers: hit.missingPeers, + finishing: (async () => {})(), + resolvedPeers: hit.resolvedPeers, + } + } return { missingPeers: hit.missingPeers, finishing: (async () => { @@ -398,164 +439,169 @@ async function resolvePeersOfNode ( } } - const { - resolvedPeers: unknownResolvedPeersOfChildren, - missingPeers: missingPeersOfChildren, - finishing, - } = await resolvePeersOfChildren(children, parentPkgs, ctx) - - const { resolvedPeers, missingPeers } = Object.keys(resolvedPackage.peerDependencies).length === 0 - ? { resolvedPeers: new Map(), missingPeers: new Set() } - : _resolvePeers({ - currentDepth: node.depth, - dependenciesTree: ctx.dependenciesTree, - lockfileDir: ctx.lockfileDir, - nodeId, - parentPkgs, - peerDependencyIssues: ctx.peerDependencyIssues, - resolvedPackage, - rootDir: ctx.rootDir, - }) + return async () => { + const { + resolvedPeers: unknownResolvedPeersOfChildren, + missingPeers: missingPeersOfChildren, + finishing, + } = await resolvePeersOfChildren(children, parentPkgs, ctx) - const allResolvedPeers = unknownResolvedPeersOfChildren - for (const [k, v] of resolvedPeers) { - allResolvedPeers.set(k, v) - } - allResolvedPeers.delete(node.resolvedPackage.name) + const { resolvedPeers, missingPeers } = Object.keys(resolvedPackage.peerDependencies).length === 0 + ? { resolvedPeers: new Map(), missingPeers: new Set() } + : _resolvePeers({ + currentDepth: node.depth, + dependenciesTree: ctx.dependenciesTree, + lockfileDir: ctx.lockfileDir, + nodeId, + parentPkgs, + peerDependencyIssues: ctx.peerDependencyIssues, + resolvedPackage, + rootDir: ctx.rootDir, + }) - const allMissingPeers = new Set() - for (const peer of missingPeersOfChildren) { - allMissingPeers.add(peer) - } - for (const peer of missingPeers) { - allMissingPeers.add(peer) - } + const allResolvedPeers = unknownResolvedPeersOfChildren + for (const [k, v] of resolvedPeers) { + allResolvedPeers.set(k, v) + } + allResolvedPeers.delete(node.resolvedPackage.name) - let cache: PeersCacheItem - const isPure = allResolvedPeers.size === 0 && allMissingPeers.size === 0 - if (isPure) { - ctx.purePkgs.add(resolvedPackage.depPath) - } else { - cache = { - missingPeers: allMissingPeers, - depPath: pDefer(), - resolvedPeers: allResolvedPeers, + const allMissingPeers = new Set() + for (const peer of missingPeersOfChildren) { + allMissingPeers.add(peer) + } + for (const peer of missingPeers) { + allMissingPeers.add(peer) } - if (ctx.peersCache.has(resolvedPackage.depPath)) { - ctx.peersCache.get(resolvedPackage.depPath)!.push(cache) + + let cache: PeersCacheItem + const isPure = allResolvedPeers.size === 0 && allMissingPeers.size === 0 + if (isPure) { + ctx.purePkgs.add(resolvedPackage.depPath) } else { - ctx.peersCache.set(resolvedPackage.depPath, [cache]) - } - } - - let calculateDepPathIfNeeded: CalculateDepPath | undefined - if (allResolvedPeers.size === 0) { - addDepPathToGraph(resolvedPackage.depPath) - } else { - const peerIds: PeerId[] = [] - const pendingPeerNodeIds: string[] = [] - for (const [alias, peerNodeId] of allResolvedPeers.entries()) { - if (peerNodeId.startsWith('link:')) { - const linkedDir = peerNodeId.slice(5) - peerIds.push({ - name: alias, - version: filenamify(linkedDir, { replacement: '+' }), - }) - continue + cache = { + missingPeers: allMissingPeers, + depPath: pDefer(), + resolvedPeers: allResolvedPeers, } - const peerDepPath = ctx.pathsByNodeId.get(peerNodeId) - if (peerDepPath) { - peerIds.push(peerDepPath) - continue + if (ctx.peersCache.has(resolvedPackage.depPath)) { + ctx.peersCache.get(resolvedPackage.depPath)!.push(cache) + } else { + ctx.peersCache.set(resolvedPackage.depPath, [cache]) } - pendingPeerNodeIds.push(peerNodeId) - } - if (pendingPeerNodeIds.length === 0) { - const peersDirSuffix = createPeersDirSuffix(peerIds) - addDepPathToGraph(`${resolvedPackage.depPath}${peersDirSuffix}`) - } else { - calculateDepPathIfNeeded = calculateDepPath.bind(null, peerIds, pendingPeerNodeIds) } - } - return { - resolvedPeers: allResolvedPeers, - missingPeers: allMissingPeers, - calculateDepPath: calculateDepPathIfNeeded, - finishing, - } - - async function calculateDepPath ( - peerIds: PeerId[], - pendingPeerNodeIds: string[], - cycles: string[][] - ): Promise { - const cyclicPeerNodeIds = new Set() - for (const cycle of cycles) { - if (cycle.includes(nodeId)) { - for (const peerNodeId of cycle) { - cyclicPeerNodeIds.add(peerNodeId) + let calculateDepPathIfNeeded: CalculateDepPath | undefined + if (allResolvedPeers.size === 0) { + addDepPathToGraph(resolvedPackage.depPath) + } else { + const peerIds: PeerId[] = [] + const pendingPeerNodeIds: string[] = [] + for (const [alias, peerNodeId] of allResolvedPeers.entries()) { + if (peerNodeId.startsWith('link:')) { + const linkedDir = peerNodeId.slice(5) + peerIds.push({ + name: alias, + version: filenamify(linkedDir, { replacement: '+' }), + }) + continue } + const peerDepPath = ctx.pathsByNodeId.get(peerNodeId) + if (peerDepPath) { + peerIds.push(peerDepPath) + continue + } + pendingPeerNodeIds.push(peerNodeId) } - } - const peersDirSuffix = createPeersDirSuffix([ - ...peerIds, - ...await Promise.all(pendingPeerNodeIds - .map(async (peerNodeId) => { - if (cyclicPeerNodeIds.has(peerNodeId)) { - const { name, version } = (ctx.dependenciesTree.get(peerNodeId)!.resolvedPackage as T) - return `${name}@${version}` - } - return ctx.pathsByNodeIdPromises.get(peerNodeId)!.promise - }) - ), - ]) - addDepPathToGraph(`${resolvedPackage.depPath}${peersDirSuffix}`) - } - - function addDepPathToGraph (depPath: string): void { - cache?.depPath.resolve(depPath) - ctx.pathsByNodeId.set(nodeId, depPath) - ctx.pathsByNodeIdPromises.get(nodeId)!.resolve(depPath) - if (ctx.depPathsByPkgId != null) { - if (!ctx.depPathsByPkgId.has(resolvedPackage.depPath)) { - ctx.depPathsByPkgId.set(resolvedPackage.depPath, new Set([depPath])) + if (pendingPeerNodeIds.length === 0) { + const peersDirSuffix = createPeersDirSuffix(peerIds) + addDepPathToGraph(`${resolvedPackage.depPath}${peersDirSuffix}`) } else { - ctx.depPathsByPkgId.get(resolvedPackage.depPath)!.add(depPath) + calculateDepPathIfNeeded = calculateDepPath.bind(null, peerIds, pendingPeerNodeIds) } } - const peerDependencies = { ...resolvedPackage.peerDependencies } - if (!ctx.depGraph[depPath] || ctx.depGraph[depPath].depth > node.depth) { - const modules = path.join(ctx.virtualStoreDir, depPathToFilename(depPath, ctx.virtualStoreDirMaxLength), 'node_modules') - const dir = path.join(modules, resolvedPackage.name) - const transitivePeerDependencies = new Set() - for (const unknownPeer of allResolvedPeers.keys()) { - if (!peerDependencies[unknownPeer]) { - transitivePeerDependencies.add(unknownPeer) + return { + resolvedPeers: allResolvedPeers, + missingPeers: allMissingPeers, + calculateDepPath: calculateDepPathIfNeeded, + finishing, + } + + async function calculateDepPath ( + peerIds: PeerId[], + pendingPeerNodeIds: string[], + cycles: string[][] + ): Promise { + const cyclicPeerNodeIds = new Set() + for (const cycle of cycles) { + if (cycle.includes(nodeId)) { + for (const peerNodeId of cycle) { + cyclicPeerNodeIds.add(peerNodeId) + } } } - for (const unknownPeer of missingPeersOfChildren) { - if (!peerDependencies[unknownPeer]) { - transitivePeerDependencies.add(unknownPeer) + const peersDirSuffix = createPeersDirSuffix([ + ...peerIds, + ...await Promise.all(pendingPeerNodeIds + .map(async (peerNodeId) => { + if (cyclicPeerNodeIds.has(peerNodeId)) { + const { name, version } = (ctx.dependenciesTree.get(peerNodeId)!.resolvedPackage as T) + return `${name}@${version}` + } + return ctx.pathsByNodeIdPromises.get(peerNodeId)!.promise + }) + ), + ]) + addDepPathToGraph(`${resolvedPackage.depPath}${peersDirSuffix}`) + } + + function addDepPathToGraph (depPath: string): void { + cache?.depPath.resolve(depPath) + if (cache) { + cache.depPath.value = depPath + } + ctx.pathsByNodeId.set(nodeId, depPath) + ctx.pathsByNodeIdPromises.get(nodeId)!.resolve(depPath) + if (ctx.depPathsByPkgId != null) { + if (!ctx.depPathsByPkgId.has(resolvedPackage.depPath)) { + ctx.depPathsByPkgId.set(resolvedPackage.depPath, new Set([depPath])) + } else { + ctx.depPathsByPkgId.get(resolvedPackage.depPath)!.add(depPath) } } - ctx.depGraph[depPath] = { - ...(node.resolvedPackage as T), - children: Object.assign( - getPreviouslyResolvedChildren(nodeId, ctx.dependenciesTree), - children, - Object.fromEntries(resolvedPeers.entries()) - ), - depPath, - depth: node.depth, - dir, - installable: node.installable, - isPure, - modules, - peerDependencies, - transitivePeerDependencies, - resolvedPeerNames: new Set(allResolvedPeers.keys()), + const peerDependencies = { ...resolvedPackage.peerDependencies } + if (!ctx.depGraph[depPath] || ctx.depGraph[depPath].depth > node.depth) { + const modules = path.join(ctx.virtualStoreDir, depPathToFilename(depPath, ctx.virtualStoreDirMaxLength), 'node_modules') + const dir = path.join(modules, resolvedPackage.name) + + const transitivePeerDependencies = new Set() + for (const unknownPeer of allResolvedPeers.keys()) { + if (!peerDependencies[unknownPeer]) { + transitivePeerDependencies.add(unknownPeer) + } + } + for (const unknownPeer of missingPeersOfChildren) { + if (!peerDependencies[unknownPeer]) { + transitivePeerDependencies.add(unknownPeer) + } + } + ctx.depGraph[depPath] = { + ...(node.resolvedPackage as T), + children: Object.assign( + getPreviouslyResolvedChildren(nodeId, ctx.dependenciesTree), + children, + Object.fromEntries(resolvedPeers.entries()) + ), + depPath, + depth: node.depth, + dir, + installable: node.installable, + isPure, + modules, + peerDependencies, + transitivePeerDependencies, + resolvedPeerNames: new Set(allResolvedPeers.keys()), + } } } } @@ -597,6 +643,8 @@ async function resolvePeersOfChildren ( }, parentPkgs: ParentRefs, ctx: ResolvePeersContext & { + allPeers: Set + getParentPkgs: Record peerDependencyIssues: Pick peersCache: PeersCache virtualStoreDir: string @@ -626,13 +674,55 @@ async function resolvePeersOfChildren ( const calculateDepPaths: CalculateDepPath[] = [] const graph = [] const finishingList: FinishingResolutionPromise[] = [] + const postponed = [] + const parentDepPaths: Record = {} + for (const [name, parentPkg] of Object.entries(parentPkgs)) { + if (!ctx.allPeers.has(name)) continue + if (parentPkg.nodeId) { + parentDepPaths[name] = (ctx.dependenciesTree.get(parentPkg.nodeId)!.resolvedPackage as T).depPath + } else { + parentDepPaths[name] = parentPkg.version + } + } + for (const childNodeId of nodeIds) { + // TODO: ONLY SELECT KNOWN PEERS + ctx.getParentPkgs[childNodeId] = () => parentDepPaths + } for (const childNodeId of nodeIds) { + const result = resolvePeersOfNode(childNodeId, parentPkgs, ctx) // eslint-disable-line no-await-in-loop + if (typeof result === 'function') { + postponed.push({ childNodeId, resolvePeers: result }) + continue + } + const { + resolvedPeers, + missingPeers, + calculateDepPath, + finishing, + } = result + if (finishing) { + finishingList.push(finishing) + } + if (calculateDepPath) { + calculateDepPaths.push(calculateDepPath) + } + const edges = [] + for (const [peerName, peerNodeId] of resolvedPeers) { + allResolvedPeers.set(peerName, peerNodeId) + edges.push(peerNodeId) + } + graph.push([childNodeId, edges]) + for (const missingPeer of missingPeers) { + allMissingPeers.add(missingPeer) + } + } + for (const { childNodeId, resolvePeers } of postponed) { const { resolvedPeers, missingPeers, calculateDepPath, finishing, - } = await resolvePeersOfNode(childNodeId, parentPkgs, ctx) // eslint-disable-line no-await-in-loop + } = await resolvePeers() if (finishing) { finishingList.push(finishing) }