fix: limit packument cache size based on heap size

DEPENDENCIES.md

@@ -583,6 +583,7 @@ graph LR;
   npmcli-arborist-->isaacs-string-locale-compare["@isaacs/string-locale-compare"];
   npmcli-arborist-->json-parse-even-better-errors;
   npmcli-arborist-->json-stringify-nice;
+  npmcli-arborist-->lru-cache;
   npmcli-arborist-->minify-registry-metadata;
   npmcli-arborist-->minimatch;
   npmcli-arborist-->nock;

smoke-tests/test/fixtures/setup.js

@@ -74,7 +74,7 @@ const getCleanPaths = async () => {
 }
 
 module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy = false } = {}) => {
-  const debugLog = debug || CI ? (...a) => console.error(...a) : () => {}
+  const debugLog = debug || CI ? (...a) => t.comment(...a) : () => {}
   const cleanPaths = await getCleanPaths()
 
   // setup fixtures
@@ -158,7 +158,7 @@ module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy
     log(`${spawnCmd} ${spawnArgs.join(' ')}`)
     log('-'.repeat(40))
 
-    const { stderr, stdout } = await spawn(spawnCmd, spawnArgs, {
+    const p = spawn(spawnCmd, spawnArgs, {
       cwd,
       env: {
         ...getEnvPath(),
@@ -169,10 +169,20 @@ module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy
       ...opts,
     })
 
-    log(stderr)
-    log('-'.repeat(40))
-    log(stdout)
-    log('='.repeat(40))
+    // In debug mode, stream stdout and stderr to console so we can debug hanging processes
+    if (debug) {
+      p.process.stdout.on('data', (c) => log('STDOUT: ' + c.toString().trim()))
+      p.process.stderr.on('data', (c) => log('STDERR: ' + c.toString().trim()))
+    }
+
+    const { stdout, stderr } = await p
+    // If not in debug mode, print full stderr and stdout contents separately
+    if (!debug) {
+      log(stderr)
+      log('-'.repeat(40))
+      log(stdout)
+      log('='.repeat(40))
+    }
 
     return { stderr, stdout }
   }

smoke-tests/test/large-install.js

@@ -5,18 +5,20 @@ const setup = require('./fixtures/setup.js')
 const getFixture = (p) => require(
   path.resolve(__dirname, './fixtures/large-install', p))
 
-const runTest = async (t) => {
+const runTest = async (t, { lowMemory } = {}) => {
   const { npm } = await setup(t, {
     // This test relies on the actual production registry
     mockRegistry: false,
     testdir: {
       project: {
         'package.json': getFixture('package.json'),
-        'package-lock.json': getFixture('package-lock.json'),
+        ...lowMemory ? {} : { 'package-lock.json': getFixture('package-lock.json') },
       },
     },
   })
-  return npm('install', '--no-audit', '--no-fund')
+  return npm('install', '--no-audit', '--no-fund', {
+    env: lowMemory ? { NODE_OPTIONS: '--max-old-space-size=500' } : {},
+  })
 }
 
 // This test was originally created for https://github.com/npm/cli/issues/6763
@@ -31,3 +33,9 @@ t.test('large install', async t => {
   // installs the same number of packages.
   t.match(stdout, `added 126${process.platform === 'linux' ? 4 : 5} packages in`)
 })
+
+t.test('large install, no lock and low memory', async t => {
+  // Run the same install but with no lockfile and constrained max-old-space-size
+  const { stdout } = await runTest(t, { lowMemory: true })
+  t.match(stdout, /added \d+ packages/)
+})

smoke-tests/test/npm-replace-global.js

@@ -125,7 +125,7 @@ t.test('publish and replace global self', async t => {
     await npmPackage({
       manifest: { packuments: [publishedPackument] },
       tarballs: { [version]: tarball },
-      times: 2,
+      times: 3,
     })
     await fs.rm(cache, { recursive: true, force: true })
     await useNpm('install', 'npm@latest', '--global')

test/lib/commands/audit.js

@@ -184,6 +184,7 @@ t.test('audit fix - bulk endpoint', async t => {
     tarballs: {
       '1.0.1': path.join(npm.prefix, 'test-dep-a-fixed'),
     },
+    times: 2,
   })
   const advisory = registry.advisory({ id: 100, vulnerable_versions: '1.0.0' })
   registry.nock.post('/-/npm/v1/security/advisories/bulk', body => {

workspaces/arborist/lib/arborist/index.js

@@ -31,10 +31,10 @@ const { homedir } = require('os')
 const { depth } = require('treeverse')
 const mapWorkspaces = require('@npmcli/map-workspaces')
 const { log, time } = require('proc-log')
-
 const { saveTypeMap } = require('../add-rm-pkg-deps.js')
 const AuditReport = require('../audit-report.js')
 const relpath = require('../relpath.js')
+const PackumentCache = require('../packument-cache.js')
 
 const mixins = [
   require('../tracker.js'),
@@ -82,7 +82,7 @@ class Arborist extends Base {
       installStrategy: options.global ? 'shallow' : (options.installStrategy ? options.installStrategy : 'hoisted'),
       lockfileVersion: lockfileVersion(options.lockfileVersion),
       packageLockOnly: !!options.packageLockOnly,
-      packumentCache: options.packumentCache || new Map(),
+      packumentCache: options.packumentCache || new PackumentCache(),
       path: options.path || '.',
       rebuildBundle: 'rebuildBundle' in options ? !!options.rebuildBundle : true,
       replaceRegistryHost: options.replaceRegistryHost,

workspaces/arborist/lib/node.js

@@ -119,6 +119,8 @@ class Node {
     // package's dependencies in a virtual root.
     this.sourceReference = sourceReference
 
+    // TODO if this came from pacote.manifest we don't have to do this,
+    // we can be told to skip this step
     const pkg = sourceReference ? sourceReference.package
       : normalize(options.pkg || {})
 

workspaces/arborist/lib/packument-cache.js

@@ -0,0 +1,56 @@
+const { LRUCache } = require('lru-cache')
+const { getHeapStatistics } = require('node:v8')
+const { log } = require('proc-log')
+
+class PackumentCache extends LRUCache {
+  static #heapLimit = Math.floor(getHeapStatistics().heap_size_limit)
+
+  #sizeKey
+  #disposed = new Set()
+
+  #log (...args) {
+    log.silly('packumentCache', ...args)
+  }
+
+  constructor ({
+    heapFactor = 0.25,
+    maxEntryFactor = 0.5,
+    sizeKey = '_contentLength',
+  } = {}) {
+    const maxSize = Math.floor(PackumentCache.#heapLimit * heapFactor)
+    const maxEntrySize = Math.floor(maxSize * maxEntryFactor)
+    super({
+      maxSize,
+      maxEntrySize,
+      // Don't cache if we dont know the size
+      sizeCalculation: (p) => p[sizeKey] || maxEntrySize + 1,
+      dispose: (v, k) => {
+        this.#disposed.add(k)
+        this.#log(k, 'dispose')
+      },
+    })
+    this.#sizeKey = sizeKey
+    this.#log(`heap:${PackumentCache.#heapLimit} maxSize:${maxSize} maxEntrySize:${maxEntrySize}`)
+  }
+
+  set (k, v, ...args) {
+    // we use disposed only for a logging signal if we are setting packuments that
+    // have already been evicted from the cache previously. logging here could help
+    // us tune this in the future.
+    const disposed = this.#disposed.has(k)
+    /* istanbul ignore next - this doesnt happen consistently so hard to test without resorting to unit tests  */
+    if (disposed) {
+      this.#disposed.delete(k)
+    }
+    this.#log(k, 'set', `size:${v[this.#sizeKey]} disposed:${disposed}`)
+    return super.set(k, v, ...args)
+  }
+
+  has (k, ...args) {
+    const has = super.has(k, ...args)
+    this.#log(k, `cache-${has ? 'hit' : 'miss'}`)
+    return has
+  }
+}
+
+module.exports = PackumentCache

workspaces/arborist/test/arborist/rebuild.js

@@ -197,7 +197,7 @@ t.test('verify dep flags in script environments', async t => {
     const file = resolve(path, 'node_modules', pkg, 'env')
     t.strictSame(flags, fs.readFileSync(file, 'utf8').split('\n'), pkg)
   }
-  t.strictSame(checkLogs().sort((a, b) =>
+  t.strictSame(checkLogs().filter(l => l[0] === 'info' && l[1] === 'run').sort((a, b) =>
     localeCompare(a[2], b[2]) || (typeof a[4] === 'string' ? -1 : 1)), [
     ['info', 'run', '[email protected]', 'postinstall', 'node_modules/devdep', 'node ../../env.js'],
     ['info', 'run', '[email protected]', 'postinstall', { code: 0, signal: null }],

workspaces/arborist/test/audit-report.js

@@ -222,7 +222,7 @@ t.test('audit returns an error', async t => {
   const report = await AuditReport.load(tree, arb.options)
   t.equal(report.report, null, 'did not get audit response')
   t.equal(report.size, 0, 'did not find any vulnerabilities')
-  t.match(logs, [
+  t.match(logs.filter(l => l[1].includes('audit')), [
     [
       'silly',
       'audit',