diff --git a/.changeset/cold-resolution-memory.md b/.changeset/cold-resolution-memory.md new file mode 100644 index 0000000000..17d225040d --- /dev/null +++ b/.changeset/cold-resolution-memory.md @@ -0,0 +1,6 @@ +--- +"@pnpm/resolving.npm-resolver": patch +"pnpm": patch +--- + +Reduced peak memory usage during cold-cache dependency resolution. The metadata fetch is memoized for the whole resolution phase, and it was retaining each package's raw registry response body (used only to mirror the response to disk) for that entire time. The memoized cache now holds a body-less copy, so the raw body only lives as long as the call that writes the disk mirror. On large graphs that fetch full metadata (e.g. with `minimumReleaseAge` or `trustPolicy` enabled) this cuts peak RSS by roughly 30%, back in line with pnpm 10. The resolved lockfile is unchanged. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 252177d3c2..f08b71514b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -809,9 +809,6 @@ catalogs: p-map-values: specifier: ^0.1.0 version: 0.1.0 - p-memoize: - specifier: 8.0.0 - version: 8.0.0 p-queue: specifier: ^9.3.0 version: 9.3.0 @@ -8816,9 +8813,6 @@ importers: p-limit: specifier: 'catalog:' version: 7.3.0 - p-memoize: - specifier: 'catalog:' - version: 8.0.0 parse-npm-tarball-url: specifier: 'catalog:' version: 5.0.0 @@ -15746,10 +15740,6 @@ packages: resolution: {integrity: sha512-km0sP12uE0dOZ5qP+s7kGVf07QngxyG0gS8sYFvFWhqlgzOsSy+m71aUejf/0akxj5W7gE//2G74qTv6b4iMog==} engines: {node: '>=10'} - p-memoize@8.0.0: - resolution: {integrity: sha512-jdZ10MCxavHoIHwJ5oweOtYy6ElPixEHaMkz0AuaEMovR1MRpVvYFzIEHRxgMEpXYzNpRVByFAniAzwmd1/uug==} - engines: {node: '>=20'} - p-queue@6.6.2: resolution: {integrity: sha512-RwFpb72c/BhQLEXIZ5K2e+AhgNVmIejGlTgiB9MzZ0e93GRvqZ7uSi0dvRF7/XIXDeNkra2fNHBxTyPDGySpjQ==} engines: {node: '>=8'} @@ -24015,11 +24005,6 @@ snapshots: mem: 6.1.1 mimic-fn: 3.1.0 - p-memoize@8.0.0: - dependencies: - mimic-function: 5.0.1 - type-fest: 4.41.0 - p-queue@6.6.2: dependencies: eventemitter3: 4.0.7 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index a10d815413..365fb9f613 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -263,7 +263,6 @@ catalog: p-filter: ^4.1.0 p-limit: ^7.3.0 p-map-values: ^0.1.0 - p-memoize: 8.0.0 p-queue: ^9.3.0 parse-json: ^8.3.0 parse-npm-tarball-url: ^5.0.0 diff --git a/pnpm11/resolving/npm-resolver/package.json b/pnpm11/resolving/npm-resolver/package.json index 6dd6a7cdeb..a990931fe8 100644 --- a/pnpm11/resolving/npm-resolver/package.json +++ b/pnpm11/resolving/npm-resolver/package.json @@ -55,7 +55,6 @@ "lru-cache": "catalog:", "normalize-path": "catalog:", "p-limit": "catalog:", - "p-memoize": "catalog:", "parse-npm-tarball-url": "catalog:", "path-temp": "catalog:", "ramda": "catalog:", diff --git a/pnpm11/resolving/npm-resolver/src/fetch.ts b/pnpm11/resolving/npm-resolver/src/fetch.ts index 578ec74db3..733429af66 100644 --- a/pnpm11/resolving/npm-resolver/src/fetch.ts +++ b/pnpm11/resolving/npm-resolver/src/fetch.ts @@ -38,7 +38,14 @@ interface RegistryResponse { export interface FetchMetadataResult { meta: PackageMeta - jsonText: string + /** + * The raw registry response body, used only to mirror the response to disk + * without re-serializing `meta`. A fresh fetch always sets it, but it + * reaches only the caller that initiated the request: the phase-long memo + * cache holds a body-less clone (see memoizeFetchMetadata.ts), so cache + * hits see `undefined` and the cache never pins the body. + */ + jsonText: string | undefined etag?: string notModified?: false } diff --git a/pnpm11/resolving/npm-resolver/src/index.ts b/pnpm11/resolving/npm-resolver/src/index.ts index 9b830f3884..f18957ab4c 100644 --- a/pnpm11/resolving/npm-resolver/src/index.ts +++ b/pnpm11/resolving/npm-resolver/src/index.ts @@ -43,13 +43,13 @@ import { import { resolveWorkspaceRange } from '@pnpm/workspace.range-resolver' import { LRUCache } from 'lru-cache' import normalize from 'normalize-path' -import pMemoize, { pMemoizeClear } from 'p-memoize' import { clone } from 'ramda' import semver from 'semver' import ssri from 'ssri' import versionSelectorType from 'version-selector-type' import { fetchMetadataFromFromRegistry, type FetchMetadataFromFromRegistryOptions, RegistryResponseError } from './fetch.js' +import { memoizeFetchMetadata } from './memoizeFetchMetadata.js' import { normalizeRegistryUrl } from './normalizeRegistryUrl.js' import { BUILTIN_NAMED_REGISTRIES, @@ -217,9 +217,7 @@ export function createNpmResolver ( timeout: opts.timeout ?? 60000, fetchWarnTimeoutMs: opts.fetchWarnTimeoutMs ?? 10 * 1000, // 10 sec } - const fetch = pMemoize(fetchMetadataFromFromRegistry.bind(null, fetchOpts), { - cacheKey: (...args) => JSON.stringify(args), - }) + const { fetch, clear: clearFetchCache } = memoizeFetchMetadata(fetchMetadataFromFromRegistry.bind(null, fetchOpts)) // Track ownership so `clearCache()` below only wipes the in-memory // cache when this factory created it. A caller-supplied // `opts.metaCache` may be shared with another resolver instance (or @@ -296,7 +294,7 @@ export function createNpmResolver ( if (ownsMetaCache && 'clear' in metaCache && typeof metaCache.clear === 'function') { metaCache.clear() } - pMemoizeClear(fetch) + clearFetchCache() }, } } diff --git a/pnpm11/resolving/npm-resolver/src/memoizeFetchMetadata.ts b/pnpm11/resolving/npm-resolver/src/memoizeFetchMetadata.ts new file mode 100644 index 0000000000..51780bf48d --- /dev/null +++ b/pnpm11/resolving/npm-resolver/src/memoizeFetchMetadata.ts @@ -0,0 +1,50 @@ +import type { + FetchMetadataNotModifiedResult, + FetchMetadataOptions, + FetchMetadataResult, +} from './fetch.js' + +export type FetchMetadata = (pkgName: string, opts: FetchMetadataOptions) => Promise + +export interface MemoizedFetchMetadata { + fetch: FetchMetadata + clear: () => void +} + +/** + * Memoizes metadata fetches for the whole resolution phase (cleared via + * `clear`, see `clearResolutionCache`), deduplicating concurrent and repeat + * requests for the same package. + * + * Unlike plain memoization, the cache holds a body-less clone of each result: + * `jsonText` — the raw registry response body, up to tens of MB for a popular + * package — reaches only the caller that initiated the fetch, which is the + * caller that writes the disk mirror. A phase-long cache that kept the bodies + * would pin hundreds of MB on large cold-cache graphs. A cache-hit caller + * that also writes the mirror falls back to `JSON.stringify(meta)` in + * `prepareJsonForDisk`, which is equivalent on read: `loadMeta` re-derives + * `etag` from the headers line. + * + * A rejected fetch is evicted so a transient network failure is retried by + * the next request instead of being cached for the rest of the phase. + */ +export function memoizeFetchMetadata (fetch: FetchMetadata): MemoizedFetchMetadata { + const cache = new Map>() + return { + fetch: (pkgName, opts) => { + const key = JSON.stringify([pkgName, opts]) + const cached = cache.get(key) + if (cached != null) return cached + const pending = fetch(pkgName, opts) + const bodiless = pending.then((result) => + result.notModified ? result : { ...result, jsonText: undefined } + ) + bodiless.catch(() => cache.delete(key)) + cache.set(key, bodiless) + return pending + }, + clear: () => { + cache.clear() + }, + } +} diff --git a/pnpm11/resolving/npm-resolver/src/pickPackage.ts b/pnpm11/resolving/npm-resolver/src/pickPackage.ts index 0933a4f0a6..72d0f45ed5 100644 --- a/pnpm11/resolving/npm-resolver/src/pickPackage.ts +++ b/pnpm11/resolving/npm-resolver/src/pickPackage.ts @@ -458,7 +458,7 @@ export async function pickPackage ( if (!isModifiedValid || modifiedDate > opts.publishedBy) { // Save the abbreviated metadata to the abbreviated cache before re-fetching full. if (!opts.dryRun) { - const abbreviatedJson = prepareJsonForDisk(fetchResult.meta, fetchResult.etag, fetchResult.jsonText) + const abbreviatedJson = prepareJsonForDisk(resultToSave.meta, resultToSave.etag, resultToSave.jsonText) // Fire-and-forget save to the abbreviated cache path (pkgMirror). runLimited(pkgMirror, (limit) => limit(async () => { try { diff --git a/pnpm11/resolving/npm-resolver/test/memoizeFetchMetadata.test.ts b/pnpm11/resolving/npm-resolver/test/memoizeFetchMetadata.test.ts new file mode 100644 index 0000000000..a1083636b1 --- /dev/null +++ b/pnpm11/resolving/npm-resolver/test/memoizeFetchMetadata.test.ts @@ -0,0 +1,104 @@ +import { expect, test } from '@jest/globals' +import type { PackageMeta } from '@pnpm/resolving.registry.types' + +import type { FetchMetadataResult } from '../src/fetch.js' +import { memoizeFetchMetadata } from '../src/memoizeFetchMetadata.js' + +const REGISTRY = 'https://registry.npmjs.org/' + +function fooFetchResult (): FetchMetadataResult { + return { + meta: { name: 'foo' } as PackageMeta, + jsonText: '{"name":"foo"}', + etag: '"abc"', + } +} + +test('the initiating caller receives the raw body; cache hits get a body-less clone sharing the same meta', async () => { + const result = fooFetchResult() + let calls = 0 + const { fetch } = memoizeFetchMetadata(async () => { + calls++ + return result + }) + + const first = await fetch('foo', { registry: REGISTRY }) + expect(first).toBe(result) + if (first.notModified) throw new Error('expected a fresh fetch result') + expect(first.jsonText).toBe('{"name":"foo"}') + + const second = await fetch('foo', { registry: REGISTRY }) + expect(calls).toBe(1) + if (second.notModified) throw new Error('expected a cached fetch result') + expect(second.jsonText).toBeUndefined() + expect(second.meta).toBe(result.meta) + expect(second.etag).toBe(result.etag) + // The clone keeps the original intact for the initiating caller. + expect(result.jsonText).toBe('{"name":"foo"}') +}) + +test('a caller arriving while the fetch is in flight gets the body-less clone', async () => { + let release!: (result: FetchMetadataResult) => void + const { fetch } = memoizeFetchMetadata(async () => new Promise((resolve) => { + release = resolve + })) + + const firstPromise = fetch('foo', { registry: REGISTRY }) + const secondPromise = fetch('foo', { registry: REGISTRY }) + release(fooFetchResult()) + + const [first, second] = await Promise.all([firstPromise, secondPromise]) + if (first.notModified || second.notModified) throw new Error('expected fresh fetch results') + expect(first.jsonText).toBe('{"name":"foo"}') + expect(second.jsonText).toBeUndefined() +}) + +test('requests with different options are cached separately', async () => { + const fetchedOpts: boolean[] = [] + const { fetch } = memoizeFetchMetadata(async (pkgName, opts) => { + fetchedOpts.push(opts.fullMetadata === true) + return fooFetchResult() + }) + + await fetch('foo', { registry: REGISTRY }) + await fetch('foo', { registry: REGISTRY, fullMetadata: true }) + await fetch('foo', { registry: REGISTRY }) + expect(fetchedOpts).toEqual([false, true]) +}) + +test('a rejected fetch is evicted so the next request retries', async () => { + let calls = 0 + const { fetch } = memoizeFetchMetadata(async () => { + calls++ + if (calls === 1) throw new Error('network down') + return fooFetchResult() + }) + + await expect(fetch('foo', { registry: REGISTRY })).rejects.toThrow('network down') + const retried = await fetch('foo', { registry: REGISTRY }) + if (retried.notModified) throw new Error('expected a fresh fetch result') + expect(retried.meta.name).toBe('foo') + expect(calls).toBe(2) +}) + +test('clear() empties the cache', async () => { + let calls = 0 + const { fetch, clear } = memoizeFetchMetadata(async () => { + calls++ + return fooFetchResult() + }) + + await fetch('foo', { registry: REGISTRY }) + clear() + await fetch('foo', { registry: REGISTRY }) + expect(calls).toBe(2) +}) + +test('notModified results pass through unchanged', async () => { + const { fetch } = memoizeFetchMetadata(async () => ({ notModified: true as const })) + + const first = await fetch('foo', { registry: REGISTRY }) + const second = await fetch('foo', { registry: REGISTRY }) + expect(first.notModified).toBe(true) + expect(second).toBe(first) +}) diff --git a/pnpm11/resolving/npm-resolver/test/metaCache.test.ts b/pnpm11/resolving/npm-resolver/test/metaCache.test.ts index 267362c099..66659bde0b 100644 --- a/pnpm11/resolving/npm-resolver/test/metaCache.test.ts +++ b/pnpm11/resolving/npm-resolver/test/metaCache.test.ts @@ -1,4 +1,5 @@ import { rmSync } from 'node:fs' +import { readFile } from 'node:fs/promises' import { expect, test } from '@jest/globals' import { ABBREVIATED_META_DIR } from '@pnpm/constants' @@ -17,6 +18,16 @@ import { const REGISTRY = 'https://registry.npmjs.org/' +async function readMirrorWithRetry (pkgMirror: string, attempts: number): Promise { + try { + return await readFile(pkgMirror, 'utf8') + } catch { + if (attempts <= 0) return undefined + await new Promise((resolve) => setTimeout(resolve, 10)) + return readMirrorWithRetry(pkgMirror, attempts - 1) + } +} + function fooMeta (): PackageMeta { return { name: 'foo', @@ -154,6 +165,32 @@ test('prefer-offline resolution promotes the disk-loaded packument into the in-m expect(second.pickedPackage?.version).toBe('1.0.0') }) +test('the raw response body is written verbatim to the disk mirror', async () => { + const meta = fooMeta() + // A body distinct from the compact JSON.stringify(meta) so we can prove the + // mirror is written from the raw response text, not re-serialized. + const rawBody = JSON.stringify(meta, null, 2) + const cacheDir = temporaryDirectory() + const pkgMirror = getPkgMirrorPath(cacheDir, ABBREVIATED_META_DIR, REGISTRY, 'foo') + + const ctx = { + fetch: async () => ({ meta, jsonText: rawBody, etag: undefined }), + metaCache: createMetaCache(), + cacheDir, + } + // A range spec avoids the exact-version fast path and, with no on-disk mirror, + // forces the network-fetch branch that writes the mirror and caches the meta. + const spec: RegistryPackageSpec = { type: 'range', name: 'foo', fetchSpec: '^1.0.0' } + + const res = await pickPackage(ctx, spec, { registry: REGISTRY, dryRun: false, preferredVersionSelectors: undefined }) + expect(res.pickedPackage?.version).toBe('1.0.0') + + // The mirror is written fire-and-forget, so retry until it appears. + const mirror = await readMirrorWithRetry(pkgMirror, 100) + // The body after the headers line is the raw response text, unchanged. + expect(mirror?.slice(mirror.indexOf('\n') + 1)).toBe(rawBody) +}) + test('a disk-promoted cache entry that cannot satisfy the spec falls back to the registry under prefer-offline', async () => { const staleMeta = fooMeta() const freshMeta = fooMeta()