perf: don't calculate package file paths in the store twice (#10428)

pkg-manager/core/src/install/link.ts

@@ -463,7 +463,7 @@ async function linkAllPkgs (
 
       depNode.requiresBuild = files.requiresBuild
       let sideEffectsCacheKey: string | undefined
-      if (opts.sideEffectsCacheRead && files.sideEffects && !isEmpty(files.sideEffects)) {
+      if (opts.sideEffectsCacheRead && files.sideEffectsMaps && !isEmpty(files.sideEffectsMaps)) {
         if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) {
           sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.depPath, {
             includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built

pkg-manager/headless/src/index.ts

@@ -871,7 +871,7 @@ async function linkAllPkgs (
 
       depNode.requiresBuild = filesResponse.requiresBuild
       let sideEffectsCacheKey: string | undefined
-      if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) {
+      if (opts.sideEffectsCacheRead && filesResponse.sideEffectsMaps && !isEmpty(filesResponse.sideEffectsMaps)) {
         if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) {
           sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.dir, {
             includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built

pkg-manager/headless/src/linkHoistedModules.ts

@@ -114,7 +114,7 @@ async function linkAllPkgsInOrder (
 
         depNode.requiresBuild = filesResponse.requiresBuild
         let sideEffectsCacheKey: string | undefined
-        if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) {
+        if (opts.sideEffectsCacheRead && filesResponse.sideEffectsMaps && !isEmpty(filesResponse.sideEffectsMaps)) {
           if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) {
             sideEffectsCacheKey = _calcDepState(dir, {
               includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built

store/cafs-types/src/index.ts

@@ -25,7 +25,8 @@ export interface PackageFilesResponse {
   resolvedFrom: ResolvedFrom
   filesMap: FilesMap
   packageImportMethod?: 'auto' | 'hardlink' | 'copy' | 'clone' | 'clone-or-copy'
-  sideEffects?: SideEffects
+  // Pre-calculated file location maps for side effects, avoiding recalculation during import
+  sideEffectsMaps?: Map<string, { added?: FilesMap, deleted?: string[] }>
   requiresBuild: boolean
 }
 

store/cafs/src/checkPkgFilesIntegrity.ts

@@ -1,12 +1,13 @@
 import fs from 'fs'
 import util from 'util'
-import { type PackageFiles, type PackageFileInfo, type SideEffects } from '@pnpm/cafs-types'
+import { type PackageFiles, type PackageFileInfo, type SideEffects, type FilesMap } from '@pnpm/cafs-types'
 import gfs from '@pnpm/graceful-fs'
 import { type DependencyManifest } from '@pnpm/types'
 import rimraf from '@zkochan/rimraf'
 import ssri from 'ssri'
 import { getFilePathByModeInCafs } from './getFilePathInCafs.js'
 import { parseJsonBufferSync } from './parseJson.js'
+import { readManifestFromStore } from './readManifestFromStore.js'
 
 // We track how many files were checked during installation.
 // It should be rare that a files content should be checked.
@@ -18,6 +19,8 @@ global['verifiedFileIntegrity'] = 0
 export interface VerifyResult {
   passed: boolean
   manifest?: DependencyManifest
+  filesMap: FilesMap
+  sideEffectsMaps?: Map<string, { added?: FilesMap, deleted?: string[] }>
 }
 
 export interface PackageFilesIndex {
@@ -44,21 +47,77 @@ export function checkPkgFilesIntegrity (
   const verifiedFilesCache = new Set<string>()
   const _checkFilesIntegrity = checkFilesIntegrity.bind(null, verifiedFilesCache, storeDir)
   const verified = _checkFilesIntegrity(pkgIndex.files, readManifest)
-  if (!verified) return { passed: false }
+  if (!verified.passed) return verified
+
+  const sideEffectsMaps = new Map<string, { added?: FilesMap, deleted?: string[] }>()
   if (pkgIndex.sideEffects) {
     // We verify all side effects cache. We could optimize it to verify only the side effects cache
     // that satisfies the current os/arch/platform.
     // However, it likely won't make a big difference.
-    for (const [sideEffectName, { added }] of pkgIndex.sideEffects) {
+    for (const [sideEffectName, { added, deleted }] of pkgIndex.sideEffects) {
       if (added) {
-        const { passed } = _checkFilesIntegrity(added)
-        if (!passed) {
+        const result = _checkFilesIntegrity(added)
+        if (!result.passed) {
           pkgIndex.sideEffects!.delete(sideEffectName)
+        } else {
+          sideEffectsMaps.set(sideEffectName, { added: result.filesMap, deleted })
         }
+      } else if (deleted) {
+        sideEffectsMaps.set(sideEffectName, { deleted })
       }
     }
   }
-  return verified
+
+  return {
+    ...verified,
+    sideEffectsMaps: sideEffectsMaps.size > 0 ? sideEffectsMaps : undefined,
+  }
+}
+
+/**
+ * Builds file maps from package index without verification.
+ * This is a lightweight alternative to checkPkgFilesIntegrity when verifyStoreIntegrity is disabled.
+ */
+export function buildFileMapsFromIndex (
+  storeDir: string,
+  pkgIndex: PackageFilesIndex,
+  readManifest?: boolean
+): VerifyResult {
+  const filesMap: FilesMap = new Map()
+
+  for (const [f, fstat] of pkgIndex.files) {
+    const filename = getFilePathByModeInCafs(storeDir, fstat.integrity, fstat.mode)
+    filesMap.set(f, filename)
+  }
+
+  const sideEffectsMaps = new Map<string, { added?: FilesMap, deleted?: string[] }>()
+  if (pkgIndex.sideEffects) {
+    for (const [sideEffectName, { added, deleted }] of pkgIndex.sideEffects) {
+      const sideEffectEntry: { added?: FilesMap, deleted?: string[] } = {}
+
+      if (added) {
+        const addedFilesMap: FilesMap = new Map()
+        for (const [f, fstat] of added) {
+          const filename = getFilePathByModeInCafs(storeDir, fstat.integrity, fstat.mode)
+          addedFilesMap.set(f, filename)
+        }
+        sideEffectEntry.added = addedFilesMap
+      }
+
+      if (deleted) {
+        sideEffectEntry.deleted = deleted
+      }
+
+      sideEffectsMaps.set(sideEffectName, sideEffectEntry)
+    }
+  }
+
+  return {
+    passed: true,
+    manifest: readManifest ? readManifestFromStore(storeDir, pkgIndex) : undefined,
+    filesMap,
+    sideEffectsMaps: sideEffectsMaps.size > 0 ? sideEffectsMaps : undefined,
+  }
 }
 
 function checkFilesIntegrity (
@@ -69,11 +128,15 @@ function checkFilesIntegrity (
 ): VerifyResult {
   let allVerified = true
   let manifest: DependencyManifest | undefined
+  const filesMap: FilesMap = new Map()
+
   for (const [f, fstat] of files) {
     if (!fstat.integrity) {
       throw new Error(`Integrity checksum is missing for ${f}`)
     }
     const filename = getFilePathByModeInCafs(storeDir, fstat.integrity, fstat.mode)
+    filesMap.set(f, filename)
+
     const readFile = readManifest && f === 'package.json'
     if (!readFile && verifiedFilesCache.has(filename)) continue
     const verifyResult = verifyFile(filename, fstat, readFile)
@@ -89,6 +152,7 @@ function checkFilesIntegrity (
   return {
     passed: allVerified,
     manifest,
+    filesMap,
   }
 }
 
@@ -100,7 +164,7 @@ function verifyFile (
   filename: string,
   fstat: FileInfo,
   readManifest?: boolean
-): VerifyResult {
+): Pick<VerifyResult, 'passed' | 'manifest'> {
   const currentFile = checkFile(filename, fstat.checkedAt)
   if (currentFile == null) return { passed: false }
   if (currentFile.isModified) {
@@ -125,7 +189,7 @@ export function verifyFileIntegrity (
   filename: string,
   expectedFile: FileInfo,
   readManifest?: boolean
-): VerifyResult {
+): Pick<VerifyResult, 'passed' | 'manifest'> {
   // @ts-expect-error
   global['verifiedFileIntegrity']++
   try {

store/cafs/src/index.ts

@@ -4,6 +4,7 @@ import { addFilesFromDir } from './addFilesFromDir.js'
 import { addFilesFromTarball } from './addFilesFromTarball.js'
 import {
   checkPkgFilesIntegrity,
+  buildFileMapsFromIndex,
   type PackageFilesIndex,
   type VerifyResult,
 } from './checkPkgFilesIntegrity.js'
@@ -21,6 +22,7 @@ export type { IntegrityLike } from 'ssri'
 
 export {
   checkPkgFilesIntegrity,
+  buildFileMapsFromIndex,
   readManifestFromStore,
   type FileType,
   getFilePathByModeInCafs,

store/create-cafs-store/src/index.ts

@@ -3,9 +3,8 @@ import path from 'path'
 import {
   type CafsLocker,
   createCafs,
-  getFilePathByModeInCafs,
 } from '@pnpm/store.cafs'
-import { type Cafs, type PackageFilesResponse, type SideEffectsDiff, type FilesMap } from '@pnpm/cafs-types'
+import { type Cafs, type PackageFilesResponse, type FilesMap } from '@pnpm/cafs-types'
 import { createIndexedPkgImporter } from '@pnpm/fs.indexed-pkg-importer'
 import {
   type ImportIndexedPackage,
@@ -83,10 +82,11 @@ function getFlatMap (
   filesResponse: PackageFilesResponse,
   targetEngine?: string
 ): { filesMap: FilesMap, isBuilt: boolean } {
-  if (targetEngine && filesResponse.sideEffects?.has(targetEngine)) {
-    const filesIndexWithSideEffects = applySideEffectsDiff(storeDir, filesResponse.filesMap, filesResponse.sideEffects.get(targetEngine)!)
+  if (targetEngine && filesResponse.sideEffectsMaps?.has(targetEngine)) {
+    const sideEffectMap = filesResponse.sideEffectsMaps.get(targetEngine)!
+    const filesMap = applySideEffectsDiffWithMaps(filesResponse.filesMap, sideEffectMap)
     return {
-      filesMap: filesIndexWithSideEffects,
+      filesMap,
       isBuilt: true,
     }
   }
@@ -96,12 +96,15 @@ function getFlatMap (
   }
 }
 
-function applySideEffectsDiff (storeDir: string, baseFiles: FilesMap, { added, deleted }: SideEffectsDiff): FilesMap {
+// Apply side effects when we already have file location maps (fast path)
+function applySideEffectsDiffWithMaps (
+  baseFiles: FilesMap,
+  { added, deleted }: { added?: FilesMap, deleted?: string[] }
+): FilesMap {
   const filesWithSideEffects = new Map<string, string>()
-  // Add side effect files (convert from PackageFiles metadata to file paths)
+  // Add side effect files (already have file paths)
   if (added) {
-    for (const [name, fileInfo] of added.entries()) {
-      const filePath = getFilePathByModeInCafs(storeDir, fileInfo.integrity, fileInfo.mode)
+    for (const [name, filePath] of added.entries()) {
       filesWithSideEffects.set(name, filePath)
     }
   }

worker/src/start.ts

@@ -12,11 +12,11 @@ import { hardLinkDir } from '@pnpm/fs.hard-link-dir'
 import {
   type CafsFunctions,
   checkPkgFilesIntegrity,
+  buildFileMapsFromIndex,
   createCafs,
   type PackageFilesIndex,
   type FilesIndex,
   optimisticRenameOverwrite,
-  readManifestFromStore,
   type VerifyResult,
 } from '@pnpm/store.cafs'
 import { symlinkDependencySync } from '@pnpm/symlink-dependency'
@@ -100,20 +100,6 @@ async function handleMessage (
         })
         return
       }
-      let verifyResult: VerifyResult | undefined
-      if (pkgFilesIndex.requiresBuild == null) {
-        readManifest = true
-      }
-      if (verifyStoreIntegrity) {
-        verifyResult = checkPkgFilesIntegrity(storeDir, pkgFilesIndex, readManifest)
-      } else {
-        verifyResult = {
-          passed: true,
-          manifest: readManifest ? readManifestFromStore(storeDir, pkgFilesIndex) : undefined,
-        }
-      }
-      const requiresBuild = pkgFilesIndex.requiresBuild ?? pkgRequiresBuild(verifyResult.manifest, pkgFilesIndex.files)
-
       const warnings: string[] = []
       if (expectedPkg) {
         if (
@@ -139,16 +125,18 @@ async function handleMessage (
           }
         }
       }
+      let verifyResult: VerifyResult | undefined
+      if (pkgFilesIndex.requiresBuild == null) {
+        readManifest = true
+      }
+      // Get file maps and optionally verify
+      if (verifyStoreIntegrity) {
+        verifyResult = checkPkgFilesIntegrity(storeDir, pkgFilesIndex, readManifest)
+      } else {
+        verifyResult = buildFileMapsFromIndex(storeDir, pkgFilesIndex, readManifest)
+      }
+      const requiresBuild = pkgFilesIndex.requiresBuild ?? pkgRequiresBuild(verifyResult.manifest, pkgFilesIndex.files)
 
-      // Convert PackageFiles to Map<string, string>
-      if (!cafsCache.has(storeDir)) {
-        cafsCache.set(storeDir, createCafs(storeDir))
-      }
-      const cafs = cafsCache.get(storeDir)!
-      const filesMap: FilesMap = new Map()
-      for (const [filename, fileInfo] of pkgFilesIndex.files) {
-        filesMap.set(filename, cafs.getFilePathByModeInCafs(fileInfo.integrity, fileInfo.mode))
-      }
       parentPort!.postMessage({
         status: 'success',
         warnings,
@@ -156,8 +144,8 @@ async function handleMessage (
           verified: verifyResult.passed,
           manifest: verifyResult.manifest,
           files: {
-            filesMap,
-            sideEffects: pkgFilesIndex.sideEffects,
+            filesMap: verifyResult.filesMap,
+            sideEffectsMaps: verifyResult.sideEffectsMaps,
             resolvedFrom: 'store',
             requiresBuild,
           },