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

@@ -22,8 +22,8 @@ export interface DependenciesGraph {
 }
 
 export default function buildSequence (
-  depGraph: DependenciesGraph,
-  rootDepPaths: string[],
+  depGraph: Record<string, Pick<DependenciesGraphNode, 'children' | 'requiresBuild'>>,
+  rootDepPaths: string[]
 ) {
   const nodesToBuild = new Set<string>()
   getSubgraphToBuild(depGraph, rootDepPaths, nodesToBuild, new Set<string>())
@@ -42,7 +42,7 @@ export default function buildSequence (
 }
 
 function getSubgraphToBuild (
-  graph: DependenciesGraph,
+  graph: Record<string, Pick<DependenciesGraphNode, 'children' | 'requiresBuild'>>,
   entryNodes: string[],
   nodesToBuild: Set<string>,
   walked: Set<string>
@@ -50,9 +50,6 @@ function getSubgraphToBuild (
   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) ||
@@ -64,4 +61,3 @@ function getSubgraphToBuild (
   }
   return currentShouldBeBuilt
 }
-

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'],
+  ])
+})