From 574c5cbee9b9d56f113ee7c62807535b7e260efa Mon Sep 17 00:00:00 2001 From: Victor Vlasenko Date: Wed, 27 Jan 2021 14:29:15 +0200 Subject: [PATCH] Implements multi-pass hoisting (its much slower atm) --- packages/yarnpkg-pnpify/sources/hoist.ts | 104 ++++++++++++++++------- 1 file changed, 71 insertions(+), 33 deletions(-) diff --git a/packages/yarnpkg-pnpify/sources/hoist.ts b/packages/yarnpkg-pnpify/sources/hoist.ts index 47fd42ea9bfe..e35b51d37afa 100644 --- a/packages/yarnpkg-pnpify/sources/hoist.ts +++ b/packages/yarnpkg-pnpify/sources/hoist.ts @@ -121,7 +121,10 @@ export const hoist = (tree: HoisterTree, opts: HoistOptions = {}): HoisterResult const treeCopy = cloneTree(tree, options); - hoistTo(treeCopy, [treeCopy], new Set([treeCopy.locator]), options); + let isGraphChanged = false; + do { + isGraphChanged = hoistTo(treeCopy, [treeCopy], new Set([treeCopy.locator]), options); + } while (isGraphChanged); if (options.debugLevel >= DebugLevel.PERF) console.timeEnd(`hoist`); @@ -139,29 +142,47 @@ export const hoist = (tree: HoisterTree, opts: HoistOptions = {}): HoisterResult return shrinkTree(treeCopy); }; -const getHoistedDependencies = (rootNode: HoisterWorkTree): Map => { - const hoistedDependencies = new Map(); +const getUsedDependencies = (rootNodePath: Array): Map => { + const rootNode = rootNodePath[rootNodePath.length - 1]; + const usedDependencies = new Map(rootNode.dependencies); const seenNodes = new Set(); + const useableDependencies = new Map(); + + for (const node of rootNodePath) + for (const dep of node.dependencies.values()) + useableDependencies.set(dep.name, dep); - const addHoistedDependencies = (node: HoisterWorkTree) => { + const addUsedDependencies = (node: HoisterWorkTree, useableDependencies: Map) => { if (seenNodes.has(node)) return; seenNodes.add(node); - for (const dep of node.hoistedDependencies.values()) - if (!rootNode.dependencies.has(dep.name)) - hoistedDependencies.set(dep.name, dep); + for (const dep of node.hoistedDependencies.values()) { + const useableDep = useableDependencies.get(dep.name); + if (useableDep) { + usedDependencies.set(useableDep.name, useableDep); + } + } + + const childrenUseableDependencies = new Map(useableDependencies); + + for (const dep of node.dependencies.values()) + childrenUseableDependencies.delete(dep.name); for (const dep of node.dependencies.values()) { if (!node.peerNames.has(dep.name)) { - addHoistedDependencies(dep); + addUsedDependencies(dep, childrenUseableDependencies); } } }; - addHoistedDependencies(rootNode); + for (const dep of rootNode.dependencies.values()) { + if (!rootNode.peerNames.has(dep.name)) { + addUsedDependencies(rootNode, useableDependencies); + } + } - return hoistedDependencies; + return usedDependencies; }; /** @@ -323,23 +344,26 @@ const getSortedRegularDependencies = (node: HoisterWorkTree): Set, rootNodePathLocators: Set, options: InternalHoistOptions, seenNodes: Set = new Set()) => { +const hoistTo = (tree: HoisterWorkTree, rootNodePath: Array, rootNodePathLocators: Set, options: InternalHoistOptions, seenNodes: Set = new Set()): boolean => { const rootNode = rootNodePath[rootNodePath.length - 1]; if (seenNodes.has(rootNode)) - return; + return false; seenNodes.add(rootNode); const preferenceMap = buildPreferenceMap(rootNode); const hoistIdentMap = getHoistIdentMap(rootNode, preferenceMap); - const hoistIdents = new Map(Array.from(hoistIdentMap.entries()).map(([k, v]) => [k, v[0]])); + const usedDependencies = tree == rootNode ? new Map(rootNode.dependencies) : getUsedDependencies(rootNodePath); + let wasStateChanged; - const hoistedDependencies = rootNode === tree ? new Map() : getHoistedDependencies(rootNode); + let isGraphChanged = false; - let wasStateChanged; + const hoistIdents = new Map(Array.from(hoistIdentMap.entries()).map(([k, v]) => [k, v[0]])); do { - hoistGraph(tree, rootNodePath, rootNodePathLocators, hoistedDependencies, hoistIdents, hoistIdentMap, options); + if (hoistGraph(tree, rootNodePath, rootNodePathLocators, usedDependencies, hoistIdents, hoistIdentMap, options)) + isGraphChanged = true; + wasStateChanged = false; for (const [name, idents] of hoistIdentMap) { if (idents.length > 1 && !rootNode.dependencies.has(name)) { @@ -354,13 +378,17 @@ const hoistTo = (tree: HoisterWorkTree, rootNodePath: Array, ro for (const dependency of rootNode.dependencies.values()) { if (!rootNode.peerNames.has(dependency.name) && !rootNodePathLocators.has(dependency.locator)) { rootNodePathLocators.add(dependency.locator); - hoistTo(tree, [...rootNodePath, dependency], rootNodePathLocators, options); + if (hoistTo(tree, [...rootNodePath, dependency], rootNodePathLocators, options)) + isGraphChanged = true; + rootNodePathLocators.delete(dependency.locator); } } + + return isGraphChanged; }; -const getNodeHoistInfo = (rootNodePathLocators: Set, nodePath: Array, node: HoisterWorkTree, hoistedDependencies: Map, hoistIdents: Map, hoistIdentMap: Map>, {outputReason}: {outputReason: boolean}): HoistInfo => { +const getNodeHoistInfo = (rootNodePathLocators: Set, nodePath: Array, node: HoisterWorkTree, usedDependencies: Map, hoistIdents: Map, hoistIdentMap: Map>, {outputReason}: {outputReason: boolean}): HoistInfo => { let reasonRoot; let reason: string | null = null; let dependsOn: Set | null = new Set(); @@ -384,18 +412,18 @@ const getNodeHoistInfo = (rootNodePathLocators: Set, nodePath: Array= 1; idx--) { const parent = nodePath[idx]; const parentDep = parent.dependencies.get(node.name); if (parentDep && parentDep.ident !== node.ident) { isNameAvailable = false; if (outputReason) - reason = `- filled by: ${prettyPrintLocator(parentDep!.locator)} at ${prettyPrintLocator(parent.locator)}`; + reason = `- filled by ${prettyPrintLocator(parentDep!.locator)} at ${nodePath.slice(0, idx).map(x => prettyPrintLocator(x.locator)).join(`→`)}`; break; } } @@ -447,23 +475,23 @@ const getNodeHoistInfo = (rootNodePathLocators: Set, nodePath: Array, rootNodePathLocators: Set, hoistedDependencies: Map, hoistIdents: Map, hoistIdentMap: Map>, options: InternalHoistOptions) => { +const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array, rootNodePathLocators: Set, usedDependencies: Map, hoistIdents: Map, hoistIdentMap: Map>, options: InternalHoistOptions): boolean => { const rootNode = rootNodePath[rootNodePath.length - 1]; const seenNodes = new Set(); + let isGraphChanged = false; const hoistNodeDependencies = (nodePath: Array, locatorPath: Array, parentNode: HoisterWorkTree, newNodes: Set) => { if (seenNodes.has(parentNode)) return; - const nextLocatorPath = [...locatorPath, parentNode.locator]; const dependantTree = new Map>(); const hoistInfos = new Map(); for (const subDependency of getSortedRegularDependencies(parentNode)) { - const hoistInfo = getNodeHoistInfo(rootNodePathLocators, [rootNode, ...nodePath, parentNode], subDependency, hoistedDependencies, hoistIdents, hoistIdentMap, {outputReason: options.debugLevel >= DebugLevel.REASONS}); + const hoistInfo = getNodeHoistInfo(rootNodePathLocators, [rootNode, ...nodePath, parentNode], subDependency, usedDependencies, hoistIdents, hoistIdentMap, {outputReason: options.debugLevel >= DebugLevel.REASONS}); hoistInfos.set(subDependency, hoistInfo); if (hoistInfo.isHoistable === Hoistable.DEPENDS) { @@ -492,27 +520,35 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array, for (const node of hoistInfos.keys()) { if (!unhoistableNodes.has(node)) { + isGraphChanged = true; parentNode.dependencies.delete(node.name); parentNode.hoistedDependencies.set(node.name, node); parentNode.reasons.delete(node.name); + const hoistedNode = rootNode.dependencies.get(node.name); let hoistedFrom: string | null = null; if (options.debugLevel >= DebugLevel.REASONS) - hoistedFrom = [``].concat(Array.from(locatorPath).slice(rootNodePathLocators.size).concat([parentNode.locator]).map(x => prettyPrintLocator(x))).join(`→`); + hoistedFrom = Array.from(locatorPath).concat([parentNode.locator]).map(x => prettyPrintLocator(x)).join(`→`); // Add hoisted node to root node, in case it is not already there if (!hoistedNode) { // Avoid adding other version of root node to itself if (rootNode.ident !== node.ident) { rootNode.dependencies.set(node.name, node); - if (options.debugLevel >= DebugLevel.REASONS) - node.hoistedFrom.push(hoistedFrom!); + if (node.hoistedFrom.length === 0) + if (options.debugLevel >= DebugLevel.REASONS) + node.hoistedFrom.push(hoistedFrom!); + newNodes.add(node); } } else { for (const reference of node.references) { hoistedNode.references.add(reference); - if (options.debugLevel >= DebugLevel.REASONS) { - hoistedNode.hoistedFrom.push(hoistedFrom!); + if (node.hoistedFrom.length === 0) { + if (options.debugLevel >= DebugLevel.REASONS) { + hoistedNode.hoistedFrom.push(hoistedFrom!); + } + } else if (node !== hoistedNode) { + hoistedNode.hoistedFrom.push(...node.hoistedFrom); } } } @@ -559,6 +595,8 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array, hoistNodeDependencies([], Array.from(rootNodePathLocators), decoupledDependency, nextNewNodes); } } while (nextNewNodes.size > 0); + + return isGraphChanged; }; const selfCheck = (tree: HoisterWorkTree): string => { @@ -834,7 +872,7 @@ const dumpDepTree = (tree: HoisterWorkTree) => { return ``; nodeCount++; - const dependencies = Array.from(pkg.dependencies.values()); + const dependencies = Array.from(pkg.dependencies.values()).sort((n1, n2) => n1.name.localeCompare(n2.name)); let str = ``; parents.add(pkg);