Skip to content

Commit

Permalink
fix: avoid caching manifests as promises
Browse files Browse the repository at this point in the history
Originally this was in #7468:

We backed off of it while we were rebuilding pacote's packument cache.

Now that that's done we can assess this in isolation.  I think it makes
sense.  The packument is cached here, all this is awaiting is
normalization and ssri calculation.

The only place this potentially does anything is in the premature
loading of manifests in `#buildDepStep` when looking at problem edges.
We can just wait till we need them and not throw a ton of requests in
parallel before we actually need them.

Removing the premature loading in problem edges will have to be a
separate effort, as it is somehow load bearing
  • Loading branch information
wraithgar committed May 9, 2024
1 parent d3b9587 commit 56a27fa
Showing 1 changed file with 9 additions and 13 deletions.
22 changes: 9 additions & 13 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,10 @@ 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)) {
// XXX This is somehow load bearing. This makes tests that print
// the ideal tree of a tree with tarball dependencies fail. This
// can't be changed or removed till we figure out why
// The test is named "tarball deps with transitive tarball deps"
promises.push(() =>
this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e)))
.catch(() => null)
Expand Down Expand Up @@ -1204,6 +1208,7 @@ This is a one-time fix-up, please be patient...
const options = {
...this.options,
avoid: this.#avoidRange(spec.name),
fullMetadata: true,
}
// get the intended spec and stored metadata from yarn.lock file,
// if available and valid.
Expand All @@ -1212,19 +1217,10 @@ This is a one-time fix-up, please be patient...
if (this.#manifests.has(spec.raw)) {
return this.#manifests.get(spec.raw)
} else {
const cleanRawSpec = redact(spec.rawSpec)
log.silly('fetch manifest', spec.raw.replace(spec.rawSpec, cleanRawSpec))
const o = {
...options,
fullMetadata: true,
}
const p = pacote.manifest(spec, o)
.then((mani) => {
this.#manifests.set(spec.raw, mani)
return mani
})
this.#manifests.set(spec.raw, p)
return p
log.silly('fetch manifest', spec.raw.replace(spec.rawSpec, redact(spec.rawSpec)))
const mani = await pacote.manifest(spec, options)
this.#manifests.set(spec.raw, mani)
return mani
}
}

Expand Down

0 comments on commit 56a27fa

Please # to comment.