From 6eca1726d22e677482cadacca24f645317816f6a Mon Sep 17 00:00:00 2001 From: Benjamin Weggersen Date: Sat, 11 May 2019 21:12:55 +0200 Subject: [PATCH] feat(run): Add just-in-time queue management (#2045) * Implement dynamic graph traversal * Centralize cycle warnings and don't prune centrally --- commands/run/index.js | 48 +++++++--- commands/run/package.json | 6 +- .../__tests__/package-graph.test.js | 4 + core/package-graph/index.js | 25 +++++- core/package-graph/package.json | 1 + package-lock.json | 87 ++++++++++++------- utils/batch-packages/batch-packages.js | 14 +-- utils/batch-packages/package.json | 1 - utils/query-graph/README.md | 9 ++ utils/query-graph/package.json | 36 ++++++++ utils/query-graph/query-graph.js | 76 ++++++++++++++++ 11 files changed, 245 insertions(+), 62 deletions(-) create mode 100644 utils/query-graph/README.md create mode 100644 utils/query-graph/package.json create mode 100644 utils/query-graph/query-graph.js diff --git a/commands/run/index.js b/commands/run/index.js index 9dad8b1709..2ddbf452ee 100644 --- a/commands/run/index.js +++ b/commands/run/index.js @@ -1,13 +1,13 @@ "use strict"; const pMap = require("p-map"); +const PQueue = require("p-queue"); const Command = require("@lerna/command"); const npmRunScript = require("@lerna/npm-run-script"); -const batchPackages = require("@lerna/batch-packages"); -const runParallelBatches = require("@lerna/run-parallel-batches"); const output = require("@lerna/output"); const timer = require("@lerna/timer"); +const QueryGraph = require("@lerna/query-graph"); const ValidationError = require("@lerna/validation-error"); const { getFilteredPackages } = require("@lerna/filter-options"); @@ -58,10 +58,6 @@ class RunCommand extends Command { // still exits zero, aka "ok" return false; } - - this.batchedPackages = this.toposort - ? batchPackages(this.packagesWithScript, this.options.rejectCycles) - : [this.packagesWithScript]; }); } @@ -77,10 +73,10 @@ class RunCommand extends Command { let chain = Promise.resolve(); const getElapsed = timer(); - if (this.options.parallel) { + if (this.options.parallel || !this.toposort) { chain = chain.then(() => this.runScriptInPackagesParallel()); } else { - chain = chain.then(() => this.runScriptInPackagesBatched()); + chain = chain.then(() => this.runScriptInPackagesTopological()); } if (this.bail) { @@ -130,18 +126,44 @@ class RunCommand extends Command { }; } - runScriptInPackagesBatched() { + runScriptInPackagesTopological() { + const queue = new PQueue({ concurrency: this.concurrency }); + const graph = new QueryGraph(this.packagesWithScript, this.options.rejectCycles); + const runner = this.options.stream ? pkg => this.runScriptInPackageStreaming(pkg) : pkg => this.runScriptInPackageCapturing(pkg); - return runParallelBatches(this.batchedPackages, this.concurrency, runner).then(batchedResults => - batchedResults.reduce((arr, batch) => arr.concat(batch), []) - ); + return new Promise((resolve, reject) => { + const returnValues = []; + + const queueNextAvailablePackages = () => + graph.getAvailablePackages().forEach(({ pkg, name }) => { + graph.markAsTaken(name); + + queue + .add(() => + runner(pkg) + .then(value => returnValues.push(value)) + .then(() => graph.markAsDone(pkg)) + .then(() => queueNextAvailablePackages()) + ) + .catch(reject); + }); + + queueNextAvailablePackages(); + + return queue.onIdle().then(() => resolve(returnValues)); + }); } runScriptInPackagesParallel() { - return pMap(this.packagesWithScript, pkg => this.runScriptInPackageStreaming(pkg)); + const runner = + this.options.parallel || this.options.stream + ? pkg => this.runScriptInPackageStreaming(pkg) + : pkg => this.runScriptInPackageCapturing(pkg); + + return pMap(this.packagesWithScript, runner); } runScriptInPackageStreaming(pkg) { diff --git a/commands/run/package.json b/commands/run/package.json index 39db64f061..732f721105 100644 --- a/commands/run/package.json +++ b/commands/run/package.json @@ -35,14 +35,14 @@ "populate--": true }, "dependencies": { - "@lerna/batch-packages": "file:../../utils/batch-packages", "@lerna/command": "file:../../core/command", "@lerna/filter-options": "file:../../core/filter-options", "@lerna/npm-run-script": "file:../../utils/npm-run-script", "@lerna/output": "file:../../utils/output", - "@lerna/run-parallel-batches": "file:../../utils/run-parallel-batches", + "@lerna/query-graph": "file:../../utils/query-graph", "@lerna/timer": "file:../../utils/timer", "@lerna/validation-error": "file:../../core/validation-error", - "p-map": "^1.2.0" + "p-map": "^1.2.0", + "p-queue": "^4.0.0" } } diff --git a/core/package-graph/__tests__/package-graph.test.js b/core/package-graph/__tests__/package-graph.test.js index 3c980776fe..619e4eca87 100644 --- a/core/package-graph/__tests__/package-graph.test.js +++ b/core/package-graph/__tests__/package-graph.test.js @@ -286,7 +286,9 @@ describe("PackageGraph", () => { expect(paths.size).toBe(0); expect(nodes.size).toBe(0); }); + }); + describe(".pruneCycleNodes()", () => { it("prunes direct cycles from the graph", () => { const pkgs = [ new Package( @@ -313,6 +315,7 @@ describe("PackageGraph", () => { const graph = new PackageGraph(pkgs); // deepInspect(graph); const [paths, nodes] = graph.partitionCycles(); + graph.pruneCycleNodes(nodes); // deepInspect(nodes); expect(graph.size).toBe(0); @@ -338,6 +341,7 @@ Set { const graph = new PackageGraph(pkgs); // deepInspect(graph); const [paths, nodes] = graph.partitionCycles(); + graph.pruneCycleNodes(nodes); // deepInspect(graph); // deepInspect(nodes); diff --git a/core/package-graph/index.js b/core/package-graph/index.js index c9ee9e9ba3..bcb433c590 100644 --- a/core/package-graph/index.js +++ b/core/package-graph/index.js @@ -2,6 +2,8 @@ const npa = require("npm-package-arg"); const semver = require("semver"); +const log = require("npmlog"); + const ValidationError = require("@lerna/validation-error"); const prereleaseIdFromVersion = require("@lerna/prerelease-id-from-version"); @@ -189,9 +191,10 @@ class PackageGraph extends Map { /** * Return a tuple of cycle paths and nodes, which have been removed from the graph. + * @param {!boolean} rejectCycles Whether or not to reject cycles * @returns [Set, Set] */ - partitionCycles() { + partitionCycles(rejectCycles) { const cyclePaths = new Set(); const cycleNodes = new Set(); @@ -235,13 +238,29 @@ class PackageGraph extends Map { currentNode.localDependents.forEach(visits([currentName])); }); - if (cycleNodes.size) { - this.prune(...cycleNodes); + if (cyclePaths.size) { + const cycleMessage = ["Dependency cycles detected, you should fix these!"] + .concat(Array.from(cyclePaths).map(cycle => cycle.join(" -> "))) + .join("\n"); + + if (rejectCycles) { + throw new ValidationError("ECYCLE", cycleMessage); + } + + log.warn("ECYCLE", cycleMessage); } return [cyclePaths, cycleNodes]; } + /** + * Remove cycle nodes. + * @param {Set} cycleNodes + */ + pruneCycleNodes(cycleNodes) { + return this.prune(...cycleNodes); + } + /** * Remove all candidate nodes. * @param {PackageGraphNode[]} candidates diff --git a/core/package-graph/package.json b/core/package-graph/package.json index a950ec2682..4b7cc5ef73 100644 --- a/core/package-graph/package.json +++ b/core/package-graph/package.json @@ -34,6 +34,7 @@ "@lerna/prerelease-id-from-version": "file:../../utils/prerelease-id-from-version", "@lerna/validation-error": "file:../validation-error", "npm-package-arg": "^6.1.0", + "npmlog": "^4.1.2", "semver": "^5.5.0" } } diff --git a/package-lock.json b/package-lock.json index 055d720456..bf9de75e5a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -690,7 +690,6 @@ "version": "file:utils/batch-packages", "requires": { "@lerna/package-graph": "file:core/package-graph", - "@lerna/validation-error": "file:core/validation-error", "npmlog": "^4.1.2" } }, @@ -1059,6 +1058,7 @@ "@lerna/prerelease-id-from-version": "file:utils/prerelease-id-from-version", "@lerna/validation-error": "file:core/validation-error", "npm-package-arg": "^6.1.0", + "npmlog": "^4.1.2", "semver": "^5.5.0" } }, @@ -1134,6 +1134,12 @@ "npmlog": "^4.1.2" } }, + "@lerna/query-graph": { + "version": "file:utils/query-graph", + "requires": { + "@lerna/package-graph": "file:core/package-graph" + } + }, "@lerna/resolve-symlink": { "version": "file:utils/resolve-symlink", "requires": { @@ -1154,15 +1160,15 @@ "@lerna/run": { "version": "file:commands/run", "requires": { - "@lerna/batch-packages": "file:utils/batch-packages", "@lerna/command": "file:core/command", "@lerna/filter-options": "file:core/filter-options", "@lerna/npm-run-script": "file:utils/npm-run-script", "@lerna/output": "file:utils/output", - "@lerna/run-parallel-batches": "file:utils/run-parallel-batches", + "@lerna/query-graph": "file:utils/query-graph", "@lerna/timer": "file:utils/timer", "@lerna/validation-error": "file:core/validation-error", - "p-map": "^1.2.0" + "p-map": "^1.2.0", + "p-queue": "^4.0.0" } }, "@lerna/run-lifecycle": { @@ -3187,6 +3193,11 @@ "integrity": "sha1-Cr9PHKpbyx96nYrMbepPqqBLrJs=", "dev": true }, + "eventemitter3": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-3.1.0.tgz", + "integrity": "sha512-ivIvhpq/Y0uSjcHDcOIccjmYjGLcP09MFGE7ysAwkAvkXfpZlC985pH2/ui64DKazbTW/4kN3yqozUxlXzI6cA==" + }, "exec-sh": { "version": "0.3.2", "resolved": "https://registry.npmjs.org/exec-sh/-/exec-sh-0.3.2.tgz", @@ -3648,8 +3659,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -3670,14 +3680,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -3692,20 +3700,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -3822,8 +3827,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -3835,7 +3839,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -3850,7 +3853,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -3858,14 +3860,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -3884,7 +3884,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -3965,8 +3964,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -3978,7 +3976,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -4085,8 +4082,7 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -4122,7 +4118,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -4142,7 +4137,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -4167,6 +4161,31 @@ "safe-buffer": "^5.1.2", "yallist": "^3.0.2" } + }, + "util-deprecate": { + "version": "1.0.2", + "bundled": true, + "dev": true, + "optional": true + }, + "wide-align": { + "version": "1.1.3", + "bundled": true, + "dev": true, + "optional": true, + "requires": { + "string-width": "^1.0.2 || 2" + } + }, + "wrappy": { + "version": "1.0.2", + "bundled": true, + "dev": true + }, + "yallist": { + "version": "3.0.3", + "bundled": true, + "dev": true } } }, @@ -6973,6 +6992,14 @@ "resolved": "https://registry.npmjs.org/p-pipe/-/p-pipe-1.2.0.tgz", "integrity": "sha1-SxoROZoRUgpneQ7loMHViB1r7+k=" }, + "p-queue": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-4.0.0.tgz", + "integrity": "sha512-3cRXXn3/O0o3+eVmUroJPSj/esxoEFIm0ZOno/T+NzG/VZgPOqQ8WKmlNqubSEpZmCIngEy34unkHGg83ZIBmg==", + "requires": { + "eventemitter3": "^3.1.0" + } + }, "p-reduce": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/p-reduce/-/p-reduce-1.0.0.tgz", diff --git a/utils/batch-packages/batch-packages.js b/utils/batch-packages/batch-packages.js index 44a8fa777b..c7b6ff174a 100644 --- a/utils/batch-packages/batch-packages.js +++ b/utils/batch-packages/batch-packages.js @@ -1,28 +1,18 @@ "use strict"; const log = require("npmlog"); - const PackageGraph = require("@lerna/package-graph"); -const ValidationError = require("@lerna/validation-error"); module.exports = batchPackages; function batchPackages(packagesToBatch, rejectCycles, graphType) { // create a new graph because we will be mutating it const graph = new PackageGraph(packagesToBatch, graphType); - const [cyclePaths, cycleNodes] = graph.partitionCycles(); + const [cyclePaths, cycleNodes] = graph.partitionCycles(rejectCycles); const batches = []; if (cyclePaths.size) { - const cycleMessage = ["Dependency cycles detected, you should fix these!"] - .concat(Array.from(cyclePaths).map(cycle => cycle.join(" -> "))) - .join("\n"); - - if (rejectCycles) { - throw new ValidationError("ECYCLE", cycleMessage); - } - - log.warn("ECYCLE", cycleMessage); + graph.pruneCycleNodes(cycleNodes); } while (graph.size) { diff --git a/utils/batch-packages/package.json b/utils/batch-packages/package.json index 5c95fad78b..81047f2624 100644 --- a/utils/batch-packages/package.json +++ b/utils/batch-packages/package.json @@ -32,7 +32,6 @@ }, "dependencies": { "@lerna/package-graph": "file:../../core/package-graph", - "@lerna/validation-error": "file:../../core/validation-error", "npmlog": "^4.1.2" } } diff --git a/utils/query-graph/README.md b/utils/query-graph/README.md new file mode 100644 index 0000000000..1a90ecd767 --- /dev/null +++ b/utils/query-graph/README.md @@ -0,0 +1,9 @@ +# `@lerna/query-graph` + +> An internal Lerna tool + +## Usage + +You probably shouldn't, at least directly. + +Install [lerna](https://www.npmjs.com/package/lerna) for access to the `lerna` CLI. diff --git a/utils/query-graph/package.json b/utils/query-graph/package.json new file mode 100644 index 0000000000..00cd5ed947 --- /dev/null +++ b/utils/query-graph/package.json @@ -0,0 +1,36 @@ +{ + "name": "@lerna/query-graph", + "version": "3.13.0", + "description": "An internal Lerna tool", + "keywords": [ + "lerna", + "utils" + ], + "homepage": "https://github.com/lerna/lerna/tree/master/utils/query-graph#readme", + "license": "MIT", + "author": { + "name": "Benjamin Weggersen", + "url": "https://github.com/bweggersen" + }, + "files": [ + "query-graph.js" + ], + "main": "query-graph.js", + "engines": { + "node": ">= 6.9.0" + }, + "publishConfig": { + "access": "public" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/lerna/lerna.git", + "directory": "utils/query-graph" + }, + "scripts": { + "test": "echo \"Run tests from root\" && exit 1" + }, + "dependencies": { + "@lerna/package-graph": "file:../../core/package-graph" + } +} diff --git a/utils/query-graph/query-graph.js b/utils/query-graph/query-graph.js new file mode 100644 index 0000000000..8b92e8e658 --- /dev/null +++ b/utils/query-graph/query-graph.js @@ -0,0 +1,76 @@ +"use strict"; + +const PackageGraph = require("@lerna/package-graph"); + +/** + * A mutable PackageGraph used to query for next available packages + * @constructor + * @param {!Array.} packages An array of Packages to build the graph out of + * @param {!boolean} rejectCycles Whether or not to reject cycles + */ + +class QueryGraph { + constructor(packages, rejectCycles) { + // Create dependency graph + this.graph = new PackageGraph(packages); + + // Evaluate cycles + [this.cyclePaths, this.cycleNodes] = this.graph.partitionCycles(rejectCycles); + + if (this.cyclePaths.size) { + // Find the cyclical package with the most dependents. Will be evaluated before other cyclical packages + this.cyclicalPackageWithMostDependents = Array.from(this.cycleNodes) + .sort((a, b) => b.localDependents.size - a.localDependents.size) + .shift(); + } + } + + _getNextLeaf() { + return Array.from(this.graph.values()).filter(node => node.localDependencies.size === 0); + } + + _getNextCycle() { + // If the cyclical package with the most dependents is still in the graph, we return it + if (this.graph.has(this.cyclicalPackageWithMostDependents.name)) { + return [this.graph.get(this.cyclicalPackageWithMostDependents.name)]; + } + + // Otherwise, return any package that does not depend on the pacakge referenced above + return Array.from(this.graph.values()).filter( + node => !node.localDependencies.has(this.cyclicalPackageWithMostDependents) + ); + } + + _onlyCyclesLeft() { + // Check if every remaining package is a pacakge from the cycleNodes graph + return Array.from(this.graph.values()).every(node => + Array.from(this.cycleNodes).some(cycleNode => cycleNode.name === node.name) + ); + } + + getAvailablePackages() { + // Get the next leaf nodes + const availablePackages = this._getNextLeaf(); + + if (availablePackages.length > 0) { + return availablePackages; + } + + // Or, get the next cylical packages + if (this.cyclePaths.size && this._onlyCyclesLeft()) { + return this._getNextCycle(); + } + + return []; + } + + markAsTaken(name) { + this.graph.delete(name); + } + + markAsDone(candidateNode) { + this.graph.remove(candidateNode); + } +} + +module.exports = QueryGraph;