From d3f184591af009b0b9471c1da1245c9206449f8b Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 23 Nov 2023 10:46:54 -0800 Subject: [PATCH] fix: clean up idealTree code --- .../arborist/lib/arborist/build-ideal-tree.js | 50 +++++++++++++------ workspaces/arborist/lib/arborist/rebuild.js | 2 +- workspaces/arborist/lib/arborist/reify.js | 11 ++-- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 7137dabfa5854..374fda00854b2 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -4,7 +4,7 @@ const rpj = require('read-package-json-fast') const npa = require('npm-package-arg') const pacote = require('pacote') const cacache = require('cacache') -const promiseCallLimit = require('promise-call-limit') +const { callLimit: promiseCallLimit } = require('promise-call-limit') const realpath = require('../../lib/realpath.js') const { resolve, dirname } = require('path') const treeCheck = require('../tree-check.js') @@ -56,30 +56,52 @@ const _global = Symbol.for('global') const _idealTreePrune = Symbol.for('idealTreePrune') // Push items in, pop them sorted by depth and then path +// Sorts physically shallower deps up to the front of the queue, because +// they'll affect things deeper in, then alphabetical for consistency between +// installs class DepsQueue { + // [{ sorted, items }] indexed by depth #deps = [] #sorted = true + #minDepth = 0 + #length = 0 get length () { - return this.#deps.length + return this.#length } push (item) { - if (!this.#deps.includes(item)) { - this.#sorted = false - this.#deps.push(item) + if (!this.#deps[item.depth]) { + this.#length++ + this.#deps[item.depth] = { sorted: true, items: [item] } + // no minDepth check needed, this branch is only reached when we are in + // the middle of a shallower depth and creating a new one + return + } + if (!this.#deps[item.depth].items.includes(item)) { + this.#length++ + this.#deps[item.depth].sorted = false + this.#deps[item.depth].items.push(item) + if (item.depth < this.#minDepth) { + this.#minDepth = item.depth + } } } pop () { - if (!this.#sorted) { - // sort physically shallower deps up to the front of the queue, because - // they'll affect things deeper in, then alphabetical - this.#deps.sort((a, b) => - (a.depth - b.depth) || localeCompare(a.path, b.path)) - this.#sorted = true + let depth + while (!depth?.items.length) { + depth = this.#deps[this.#minDepth] + if (!depth?.items.length) { + this.#minDepth++ + } + } + if (!depth.sorted) { + depth.items.sort((a, b) => localeCompare(a.path, b.path)) + depth.sorted = true } - return this.#deps.shift() + this.#length-- + return depth.items.shift() } } @@ -1016,7 +1038,7 @@ This is a one-time fix-up, please be patient... // may well be an optional dep that has gone missing. it'll // fail later anyway. for (const e of this.#problemEdges(placed)) { - promises.push( + promises.push(() => this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e))) .catch(er => null) ) @@ -1031,7 +1053,7 @@ This is a one-time fix-up, please be patient... } } - await Promise.all(promises) + await promiseCallLimit(promises) return this.#buildDepStep() } diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js index d502d5244bdc7..32f27a9251298 100644 --- a/workspaces/arborist/lib/arborist/rebuild.js +++ b/workspaces/arborist/lib/arborist/rebuild.js @@ -7,7 +7,7 @@ const promiseAllRejectLate = require('promise-all-reject-late') const rpj = require('read-package-json-fast') const binLinks = require('bin-links') const runScript = require('@npmcli/run-script') -const promiseCallLimit = require('promise-call-limit') +const { callLimit: promiseCallLimit } = require('promise-call-limit') const { resolve } = require('path') const { isNodeGypPackage, diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 7ce3bc2a9db1d..801712c293d51 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -31,6 +31,7 @@ const relpath = require('../relpath.js') const Diff = require('../diff.js') const retirePath = require('../retire-path.js') const promiseAllRejectLate = require('promise-all-reject-late') +const { callLimit: promiseCallLimit } = require('promise-call-limit') const optionalSet = require('../optional-set.js') const calcDepFlags = require('../calc-dep-flags.js') const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js') @@ -817,10 +818,12 @@ module.exports = cls => class Reifier extends cls { } // extract all the nodes with bundles - return promiseAllRejectLate(set.map(node => { - this[_bundleUnpacked].add(node) - return this[_reifyNode](node) - })) + return promiseCallLimit(set.map(node => { + return () => { + this[_bundleUnpacked].add(node) + return this[_reifyNode](node) + } + }), { rejectLate: true }) // then load their unpacked children and move into the ideal tree .then(nodes => promiseAllRejectLate(nodes.map(async node => {