Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modular build builds in concurrent batches #2341

Merged
merged 12 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/old-fans-invent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"modular-scripts": minor
---

`modular build` can build concurrently; `--concurrencyLevel` command added
9 changes: 9 additions & 0 deletions __fixtures__/ghost-building/packages/f/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "f",
"private": false,
"modular": {
"type": "package"
},
"main": "./src/index.ts",
"version": "1.0.0"
}
3 changes: 3 additions & 0 deletions __fixtures__/ghost-building/packages/f/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function add(a: number, b: number): number {
return a + b;
}
17 changes: 13 additions & 4 deletions docs/commands/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ Search workspaces based on their `name` field in the `package.json` and build:
[workspace](https://classic.yarnpkg.com/en/docs/cli/workspace).

Packages are always built in order of dependency (e.g. if a package `a` depends
on a package `b`, `b` is built first).
on a package `b`, `b` is built first). Packages may be built concurrently when
possible. The maximum concurrency level defaults to the number of CPUs available
on the machine, but can be set by the user (see the `--concurrencyLevel`
option).

The output directory for built artifacts is `dist/`, which has a flat structure
of modular package names. Each built app/view/package is added to the `dist/` as
Expand Down Expand Up @@ -45,9 +48,9 @@ changed. Files that have changed are calculated comparing the current state of
the repository with the branch specified by `compareBranch` or, if
`compareBranch` is not set, with the default git branch.

`--compareBranch`: Specify the comparison branch used to determine which files
have changed when using the `changed` option. If this option is used without
`changed`, the command will fail.
`--compareBranch <branchName>`: Specify the comparison branch used to determine
which files have changed when using the `changed` option. If this option is used
without `changed`, the command will fail.

`--descendants`: Build the packages specified by the `[packages...]` argument
and/or the `--changed` option and additionally build all their descendants (i.e.
Expand All @@ -58,6 +61,12 @@ and/or the `--changed` option and additionally build all their ancestors (i.e.
the packages that have a direct or indirect dependency on them) in dependency
order.

`--concurrencyLevel <level>`: Limit the concurrency of build processes that are
executed in different processes within build batches. 0 or 1 means no
concurrency. If not specified, this option defaults to
[the number of logical CPUs](https://nodejs.org/api/os.html#oscpus) available on
the machine, or 1 if that information is not available.

## Dependency selection and build order examples

We'll be using this package manifests in our Modular monorepo for the following
Expand Down
4 changes: 2 additions & 2 deletions packages/modular-scripts/src/__tests__/select.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe('select', () => {
const result = runModularPipeLogs(tempModularRepo, 'select');

expect(result.stderr).toBeFalsy();
expect(result.stdout).toContain(format(['a', 'b', 'c', 'd', 'e']));
expect(result.stdout).toContain(format(['a', 'b', 'c', 'd', 'e', 'f']));
});
});

Expand Down Expand Up @@ -307,7 +307,7 @@ describe('select in buildable order', () => {

expect(result.stderr).toBeFalsy();
expect(result.stdout).toContain(
format([['d'], ['c'], ['b'], ['a'], ['e']]),
format([['d'], ['c'], ['b'], ['a'], ['e', 'f']]),
);
});
});
Expand Down
57 changes: 56 additions & 1 deletion packages/modular-scripts/src/__tests__/selectiveBuild.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,62 @@ describe('--changed builds all the changed packages in order', () => {
const result = runModularPipeLogs(tempModularRepo, 'build');

expect(result.stderr).toBeFalsy();
expect(getBuildOrder(result.stdout)).toEqual(['d', 'c', 'b', 'a', 'e']);
expect(getBuildOrder(result.stdout)).toEqual([
'd',
'c',
'b',
'a',
'f',
'e',
]);
});

it('builds packages concurrently with a certain concurrencyLevel', () => {
const result = runModularPipeLogs(
tempModularRepo,
'build --verbose --concurrencyLevel 2',
);

expect(result.stderr).toBeFalsy();
expect(result.stdout).toContain(
'runBatch is running a batch of length 2: ["e","f"]',
);
});

it('removes concurrency with concurrencyLevel = 0', () => {
const result = runModularPipeLogs(
tempModularRepo,
'build --verbose --concurrencyLevel 0',
);

expect(result.stderr).toBeFalsy();
expect(result.stdout).not.toContain(
'runBatch is running a batch of length 2',
);
expect(result.stdout).toContain(
'runBatch is running a batch of length 1: ["e"]',
);
expect(result.stdout).toContain(
'runBatch is running a batch of length 1: ["f"]',
);
});

it('removes concurrency with concurrencyLevel = 1', () => {
const result = runModularPipeLogs(
tempModularRepo,
'build --verbose --concurrencyLevel 0',
);

expect(result.stderr).toBeFalsy();
expect(result.stdout).not.toContain(
'runBatch is running a batch of length 2',
);
expect(result.stdout).toContain(
'runBatch is running a batch of length 1: ["e"]',
);
expect(result.stdout).toContain(
'runBatch is running a batch of length 1: ["f"]',
);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('When there is a non-Modular package with a build script', () => {
expect(result.stderr).toBeFalsy();
expect(result.stdout).toContain('\nnon-modular-buildable was built\n');
expect(result.stdout).toContain(
'Building the following workspaces in order: ["non-modular-buildable","app"]',
'Building the following workspaces in order: [["non-modular-buildable"],["app"]]',
);
expect(result.stdout).toContain('Compiled successfully.');
});
Expand Down
118 changes: 90 additions & 28 deletions packages/modular-scripts/src/build-scripts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import { buildPackage } from './build-package';
import * as logger from '../utils/logger';
import actionPreflightCheck from '../utils/actionPreflightCheck';
import getWorkspaceLocation from '../utils/getLocation';
import { selectBuildableWorkspaces } from '../utils/selectWorkspaces';
import { selectParallellyBuildableWorkspaces } from '../utils/selectWorkspaces';
import { setupEnvForDirectory } from '../utils/setupEnv';
import { getAllWorkspaces } from '../utils/getAllWorkspaces';
import getModularRoot from '../utils/getModularRoot';
import execAsync from '../utils/execAsync';
import type { ModularWorkspacePackage } from '@modular-scripts/modular-types';

async function build({
packagePaths,
Expand All @@ -18,6 +19,7 @@ async function build({
changed,
compareBranch,
dangerouslyIgnoreCircularDependencies,
concurrencyLevel,
}: {
packagePaths: string[];
preserveModules: boolean;
Expand All @@ -27,6 +29,7 @@ async function build({
changed: boolean;
compareBranch?: string;
dangerouslyIgnoreCircularDependencies: boolean;
concurrencyLevel: number;
}): Promise<void> {
const isSelective =
changed || ancestors || descendants || packagePaths.length;
Expand All @@ -37,7 +40,7 @@ async function build({
// targets are either the set of what's specified in the selective options or all the packages in the monorepo
const targets = isSelective ? packagePaths : [...allWorkspacePackages.keys()];

const selectedTargets = await selectBuildableWorkspaces({
const selectedTargets = await selectParallellyBuildableWorkspaces({
targets,
changed,
compareBranch,
Expand All @@ -57,38 +60,97 @@ async function build({
)}`,
);

for (const target of selectedTargets) {
const packageInfo = allWorkspacePackages.get(target);
for (const batch of selectedTargets) {
logger.debug(
`Building batch: ${JSON.stringify(
batch,
)} with concurrency ${concurrencyLevel}`,
);
const jobBatch = batch.map((target) => {
const packageInfo = allWorkspacePackages.get(target);
if (!packageInfo) {
throw new Error(
`building ${target} failed - pacakge ${target} has no package info.`,
);
}
return {
id: packageInfo.name,
promise: () => {
return runBuildJob({
packageInfo,
preserveModules,
includePrivate,
cwd: modularRoot,
});
},
};
});
await runBatch(jobBatch, concurrencyLevel);
}
}

interface Job {
id: string;
promise: (...args: unknown[]) => Promise<void>;
}
interface BuildParameters {
packageInfo: ModularWorkspacePackage;
preserveModules: boolean;
includePrivate: boolean;
cwd: string;
}

try {
const targetDirectory = await getWorkspaceLocation(target);
await setupEnvForDirectory(targetDirectory);
if (packageInfo?.modular) {
// If it's modular, build with Modular
const targetType = packageInfo.modular.type;
if (!targetType)
throw new Error(`modular.type missing in ${target} package.json`);
async function runBuildJob({
packageInfo,
preserveModules,
includePrivate,
cwd,
}: BuildParameters) {
const target = packageInfo?.name;
try {
const targetDirectory = await getWorkspaceLocation(target);
await setupEnvForDirectory(targetDirectory);
if (packageInfo?.modular) {
// If it's modular, build with Modular
const targetType = packageInfo.modular.type;
if (!targetType)
throw new Error(`modular.type missing in ${target} package.json`);

logger.log('\nBuilding', targetType, target);
logger.log('\nBuilding', targetType, target);

if (targetType === 'app' || targetType === 'esm-view') {
await buildStandalone(target, targetType);
} else {
await buildPackage(target, preserveModules, includePrivate);
}
if (targetType === 'app' || targetType === 'esm-view') {
await buildStandalone(target, targetType);
} else {
// Otherwise, build by running the workspace's build script
// We're sure it's here because selectBuildableWorkspaces returns only buildable workspaces.
logger.log('\nBuilding non-modular package', target);
await execAsync(`yarn`, ['workspace', target, 'build'], {
cwd: modularRoot,
log: false,
});
await buildPackage(target, preserveModules, includePrivate);
}
} catch (err) {
logger.error(`building ${target} failed`);
throw err;
} else {
// Otherwise, build by running the workspace's build script
// We're sure it's here because selectParallellyBuildableWorkspaces returns only buildable workspaces.
logger.log('\nBuilding non-modular package', target);
await execAsync(`yarn`, ['workspace', target, 'build'], {
cwd,
log: false,
});
}
} catch (err) {
logger.error(`building ${target} failed`);
throw err;
}
}

async function runBatch<T extends Job>(jobs: T[], concurrency: number) {
while (jobs.length) {
// Councurrency = [0|1] means no concurrency in a user-friendly way. Since 0 would spin forever, it's defaulted to 1.
const nextConcurrentBatch = jobs.splice(0, concurrency || 1);
const ids = nextConcurrentBatch.map((j) => j.id);
const promises = nextConcurrentBatch.map((j) => j.promise);
logger.debug(
`runBatch is running a batch of length ${JSON.stringify(
ids.length,
)}: ${JSON.stringify(ids)}`,
);
// It's not Promise.all that starts the functions, but the act of invoking them, so we need to defer invocation until it's needed.
await Promise.all(promises.map((f) => f()));
}
}

Expand Down
19 changes: 18 additions & 1 deletion packages/modular-scripts/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import commander, { Option } from 'commander';
import { testOptions } from './test/jestOptions';
import actionPreflightCheck from './utils/actionPreflightCheck';
import * as logger from './utils/logger';
import { validateCompareOptions } from './utils/validateOptions';
import {
validateCompareOptions,
computeConcurrencyOption,
} from './utils/options';

import type { JSONSchemaForNPMPackageJsonFiles as PackageJson } from '@schemastore/package';
import type { TestOptions } from './test';
Expand Down Expand Up @@ -111,6 +114,10 @@ program
"Ignore circular dependency checks if your graph has one or more circular dependencies involving 'source' types, then warn. The build will still fail if circular dependencies involve more than one buildable package. Circular dependencies can be always refactored to remove cycles. This switch is dangerous and should be used sparingly and only temporarily.",
false,
)
.option(
'--concurrencyLevel <level>',
'Limit the concurrency of build processes that are executed in parallel within batches. 0 or 1 means no concurrency. Default is the number of logical CPUs.',
)
.action(
async (
packagePaths: string[],
Expand All @@ -122,12 +129,21 @@ program
ancestors: boolean;
descendants: boolean;
dangerouslyIgnoreCircularDependencies: boolean;
concurrencyLevel?: string;
},
) => {
const { default: build } = await import('./build-scripts');

validateCompareOptions(options.compareBranch, options.changed);

const concurrencyLevel = computeConcurrencyOption(
options.concurrencyLevel,
);

logger.debug(
`Running build with a concurrency level of ${concurrencyLevel}`,
);

if (options.dangerouslyIgnoreCircularDependencies) {
// Warn. Users should never use this, but if they use it, they should have cycles limited to "source" packages
// and they should do this in a temporary way (for example, to onboard large projects).
Expand All @@ -146,6 +162,7 @@ program
descendants: options.descendants,
dangerouslyIgnoreCircularDependencies:
options.dangerouslyIgnoreCircularDependencies,
concurrencyLevel,
});
},
);
Expand Down
Loading