perf: build-modules

.changeset/twenty-moons-lick.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/build-modules": patch
+"pnpm": patch
+---
+
+Improve the performance of the build sequence calculation step.

packages/build-modules/jest.config.js

@@ -0,0 +1 @@
+module.exports = require('../../jest.config')

packages/build-modules/package.json

@@ -12,10 +12,11 @@
     "node": ">=14.19"
   },
   "scripts": {
-    "lint": "eslint src/**/*.ts",
-    "test": "pnpm run compile",
+    "lint": "eslint src/**/*.ts test/**/*.ts",
+    "test": "pnpm run compile && pnpm run _test",
     "prepublishOnly": "pnpm run compile",
-    "compile": "tsc --build && pnpm run lint --fix"
+    "compile": "tsc --build && pnpm run lint --fix",
+    "_test": "jest"
   },
   "repository": "https://github.com/pnpm/pnpm/blob/main/packages/build-modules",
   "keywords": [
@@ -35,12 +36,12 @@
   "dependencies": {
     "@pnpm/calc-dep-state": "workspace:2.0.1",
     "@pnpm/core-loggers": "workspace:7.0.1",
+    "@pnpm/graph-sequencer": "1.0.0",
     "@pnpm/lifecycle": "workspace:13.0.2",
     "@pnpm/link-bins": "workspace:7.1.1",
     "@pnpm/read-package-json": "workspace:6.0.2",
     "@pnpm/store-controller-types": "workspace:13.0.1",
     "@pnpm/types": "workspace:8.0.1",
-    "@pnpm/graph-sequencer": "1.0.0",
     "ramda": "^0.27.1",
     "run-groups": "^3.0.1"
   },

packages/build-modules/src/buildSequence.ts

@@ -0,0 +1,63 @@
+import graphSequencer from '@pnpm/graph-sequencer'
+import { PackageManifest } from '@pnpm/types'
+import filter from 'ramda/src/filter'
+
+export interface DependenciesGraphNode {
+  children: {[alias: string]: string}
+  depPath: string
+  dir: string
+  fetchingBundledManifest?: () => Promise<PackageManifest>
+  filesIndexFile: string
+  hasBin: boolean
+  hasBundledDependencies: boolean
+  installable?: boolean
+  isBuilt?: boolean
+  optional: boolean
+  optionalDependencies: Set<string>
+  requiresBuild?: boolean
+}
+
+export interface DependenciesGraph {
+  [depPath: string]: DependenciesGraphNode
+}
+
+export default function buildSequence (
+  depGraph: Record<string, Pick<DependenciesGraphNode, 'children' | 'requiresBuild'>>,
+  rootDepPaths: string[]
+) {
+  const nodesToBuild = new Set<string>()
+  getSubgraphToBuild(depGraph, rootDepPaths, nodesToBuild, new Set<string>())
+  const onlyFromBuildGraph = filter((depPath: string) => nodesToBuild.has(depPath))
+  const nodesToBuildArray = Array.from(nodesToBuild)
+  const graph = new Map(
+    nodesToBuildArray
+      .map((depPath) => [depPath, onlyFromBuildGraph(Object.values(depGraph[depPath].children))])
+  )
+  const graphSequencerResult = graphSequencer({
+    graph,
+    groups: [nodesToBuildArray],
+  })
+  const chunks = graphSequencerResult.chunks as string[][]
+  return chunks
+}
+
+function getSubgraphToBuild (
+  graph: Record<string, Pick<DependenciesGraphNode, 'children' | 'requiresBuild'>>,
+  entryNodes: string[],
+  nodesToBuild: Set<string>,
+  walked: Set<string>
+) {
+  let currentShouldBeBuilt = false
+  for (const depPath of entryNodes) {
+    if (!graph[depPath]) continue // packages that are already in node_modules are skipped
+    if (walked.has(depPath)) continue
+    walked.add(depPath)
+    const childShouldBeBuilt = getSubgraphToBuild(graph, Object.values(graph[depPath].children), nodesToBuild, walked) ||
+      graph[depPath].requiresBuild
+    if (childShouldBeBuilt) {
+      nodesToBuild.add(depPath)
+      currentShouldBeBuilt = true
+    }
+  }
+  return currentShouldBeBuilt
+}

packages/build-modules/src/index.ts

@@ -6,10 +6,9 @@ import linkBins, { linkBinsOfPackages } from '@pnpm/link-bins'
 import logger from '@pnpm/logger'
 import { fromDir as readPackageFromDir } from '@pnpm/read-package-json'
 import { StoreController } from '@pnpm/store-controller-types'
-import { DependencyManifest, PackageManifest } from '@pnpm/types'
+import { DependencyManifest } from '@pnpm/types'
 import runGroups from 'run-groups'
-import graphSequencer from '@pnpm/graph-sequencer'
-import filter from 'ramda/src/filter'
+import buildSequence, { DependenciesGraph, DependenciesGraphNode } from './buildSequence'
 
 export { DepsStateCache }
 
@@ -38,21 +37,9 @@ export default async (
 ) => {
   const warn = (message: string) => logger.warn({ message, prefix: opts.lockfileDir })
   // postinstall hooks
-  const nodesToBuild = new Set<string>()
-  getSubgraphToBuild(depGraph, rootDepPaths, nodesToBuild, new Set<string>())
-  const onlyFromBuildGraph = filter((depPath: string) => nodesToBuild.has(depPath))
 
-  const nodesToBuildArray = Array.from(nodesToBuild)
-  const graph = new Map(
-    nodesToBuildArray
-      .map((depPath) => [depPath, onlyFromBuildGraph(Object.values(depGraph[depPath].children))])
-  )
-  const graphSequencerResult = graphSequencer({
-    graph,
-    groups: [nodesToBuildArray],
-  })
-  const chunks = graphSequencerResult.chunks as string[][]
   const buildDepOpts = { ...opts, warn }
+  const chunks = buildSequence(depGraph, rootDepPaths)
   const groups = chunks.map((chunk) => {
     chunk = chunk.filter((depPath) => depGraph[depPath].requiresBuild && !depGraph[depPath].isBuilt)
     if (opts.depsToBuild != null) {
@@ -145,49 +132,6 @@ async function buildDependency (
   }
 }
 
-function getSubgraphToBuild (
-  graph: DependenciesGraph,
-  entryNodes: string[],
-  nodesToBuild: Set<string>,
-  walked: Set<string>
-) {
-  let currentShouldBeBuilt = false
-  for (const depPath of entryNodes) {
-    if (!graph[depPath]) continue // packages that are already in node_modules are skipped
-    if (nodesToBuild.has(depPath)) {
-      currentShouldBeBuilt = true
-    }
-    if (walked.has(depPath)) continue
-    walked.add(depPath)
-    const childShouldBeBuilt = getSubgraphToBuild(graph, Object.values(graph[depPath].children), nodesToBuild, walked) ||
-      graph[depPath].requiresBuild
-    if (childShouldBeBuilt) {
-      nodesToBuild.add(depPath)
-      currentShouldBeBuilt = true
-    }
-  }
-  return currentShouldBeBuilt
-}
-
-export interface DependenciesGraphNode {
-  children: {[alias: string]: string}
-  depPath: string
-  dir: string
-  fetchingBundledManifest?: () => Promise<PackageManifest>
-  filesIndexFile: string
-  hasBin: boolean
-  hasBundledDependencies: boolean
-  installable?: boolean
-  isBuilt?: boolean
-  optional: boolean
-  optionalDependencies: Set<string>
-  requiresBuild?: boolean
-}
-
-export interface DependenciesGraph {
-  [depPath: string]: DependenciesGraphNode
-}
-
 export async function linkBinsOfDependencies (
   depNode: DependenciesGraphNode,
   depGraph: DependenciesGraph,

packages/build-modules/test/buildSequence.test.ts

@@ -0,0 +1,81 @@
+import buildSequence from '../lib/buildSequence'
+
+test('buildSequence() test 1', () => {
+  const chunks = buildSequence({
+    '/a/1.0.0': {
+      children: {
+        c: '/c/1.0.0',
+      },
+      requiresBuild: true,
+    },
+    '/b/1.0.0': {
+      children: {
+        c: '/c/1.0.0',
+      },
+      requiresBuild: true,
+    },
+    '/c/1.0.0': {
+      children: {},
+      requiresBuild: true,
+    },
+  }, ['/a/1.0.0', '/b/1.0.0'])
+  expect(chunks).toStrictEqual([
+    ['/c/1.0.0'],
+    ['/a/1.0.0', '/b/1.0.0'],
+  ])
+})
+
+test('buildSequence() test 2', () => {
+  const chunks = buildSequence({
+    '/a/1.0.0': {
+      children: {
+        c: '/c/1.0.0',
+      },
+      requiresBuild: true,
+    },
+    '/b/1.0.0': {
+      children: {
+        c: '/c/1.0.0',
+      },
+    },
+    '/c/1.0.0': {
+      children: {},
+      requiresBuild: true,
+    },
+  }, ['/a/1.0.0', '/b/1.0.0'])
+  expect(chunks).toStrictEqual([
+    ['/c/1.0.0'],
+    ['/a/1.0.0'],
+  ])
+})
+
+test('buildSequence() test 3', () => {
+  const chunks = buildSequence({
+    '/a/1.0.0': {
+      children: {
+        c: '/c/1.0.0',
+      },
+      requiresBuild: true,
+    },
+    '/b/1.0.0': {
+      children: {
+        d: '/d/1.0.0',
+      },
+    },
+    '/c/1.0.0': {
+      children: {},
+      requiresBuild: true,
+    },
+    '/d/1.0.0': {
+      children: {
+        c: '/c/1.0.0',
+      },
+      requiresBuild: true,
+    },
+  }, ['/a/1.0.0', '/b/1.0.0'])
+  expect(chunks).toStrictEqual([
+    ['/c/1.0.0'],
+    ['/a/1.0.0', '/d/1.0.0'],
+    ['/b/1.0.0'],
+  ])
+})