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

chore(refactor): explore parallelization of builds #8962

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 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
8 changes: 8 additions & 0 deletions .changeset/mighty-windows-wink.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"app-builder-lib": patch
"builder-util": patch
"dmg-builder": patch
"electron-builder-squirrel-windows": patch
---

chore(refactor): enable parallel packaging of archs and targets with `concurrency` config prop
6 changes: 5 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ jobs:
- snapTest,debTest,fpmTest,protonTest
- winPackagerTest,winCodeSignTest,webInstallerTest
- oneClickInstallerTest,assistedInstallerTest
- concurrentBuildsTest
steps:
- name: Checkout code repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
Expand Down Expand Up @@ -169,6 +170,7 @@ jobs:
- winCodeSignTest,differentialUpdateTest,squirrelWindowsTest
- appxTest,msiTest,portableTest,assistedInstallerTest,protonTest
- BuildTest,oneClickInstallerTest,winPackagerTest,nsisUpdaterTest,webInstallerTest
- concurrentBuildsTest
steps:
- name: Checkout code repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
Expand Down Expand Up @@ -196,6 +198,7 @@ jobs:
- oneClickInstallerTest,assistedInstallerTest
- winPackagerTest,winCodeSignTest,webInstallerTest
- masTest,dmgTest,filesTest,macPackagerTest,differentialUpdateTest,macArchiveTest
- concurrentBuildsTest
steps:
- name: Checkout code repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
Expand All @@ -206,10 +209,11 @@ jobs:
cache-path: ~/Library/Caches/electron
cache-key: v-23.3.10-macos-electron

- name: Install pwsh and wine via brew
- name: Install toolset via brew
run: |
brew install powershell/tap/powershell
brew install --cask wine-stable
brew install rpm

- name: Test
run: pnpm ci:test
Expand Down
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"console": "integratedTerminal",
"internalConsoleOptions": "openOnFirstSessionStart",
"env": {
"TEST_FILES": "BuildTest"
"TEST_FILES": "macPackagerTest",
"UPDATE_SNAPSHOT": "false"
}
}
]
Expand Down
25 changes: 25 additions & 0 deletions packages/app-builder-lib/scheme.json
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,20 @@
],
"type": "object"
},
"Concurrency": {
"additionalProperties": false,
"properties": {
"jobs": {
"default": 1,
"description": "The maximum number of concurrent jobs to run.",
"type": "number"
}
},
"required": [
"jobs"
],
"type": "object"
},
"CustomNsisBinary": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -6924,6 +6938,17 @@
"default": "normal",
"description": "The compression level. If you want to rapidly test build, `store` can reduce build time significantly. `maximum` doesn't lead to noticeable size difference, but increase build time."
},
"concurrency": {
"anyOf": [
{
"$ref": "#/definitions/Concurrency"
},
{
"type": "null"
}
],
"description": "[Experimental] Configuration for concurrent builds."
},
"copyright": {
"default": "Copyright © year ${author}",
"description": "The human-readable copyright line for the app.",
Expand Down
14 changes: 14 additions & 0 deletions packages/app-builder-lib/src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,13 @@ export interface CommonConfiguration {
* Ref: https://github.com/electron/fuses
*/
readonly electronFuses?: FuseOptionsV1 | null

/**
* [Experimental] Configuration for concurrent builds.
*/
readonly concurrency?: Concurrency | null
}

export interface Configuration extends CommonConfiguration, PlatformSpecificBuildOptions, Hooks {
/**
* Whether to use [electron-compile](http://github.com/electron/electron-compile) to compile app. Defaults to `true` if `electron-compile` in the dependencies. And `false` if in the `devDependencies` or doesn't specified.
Expand Down Expand Up @@ -439,3 +445,11 @@ export interface FuseOptionsV1 {
*/
resetAdHocDarwinSignature?: boolean
}

export interface Concurrency {
/**
* The maximum number of concurrent jobs to run.
* @default 1
*/
jobs: number
}
11 changes: 7 additions & 4 deletions packages/app-builder-lib/src/core.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Arch, archFromString, ArchType } from "builder-util"
import { AllPublishOptions, Nullish } from "builder-util-runtime"
import { Arch, archFromString, ArchType, AsyncTaskManager } from "builder-util"
import { AllPublishOptions, CancellationToken, Nullish } from "builder-util-runtime"

// https://github.com/YousefED/typescript-json-schema/issues/80
export type Publish = AllPublishOptions | Array<AllPublishOptions> | null
Expand Down Expand Up @@ -75,6 +75,9 @@ export abstract class Target {
abstract readonly outDir: string
abstract readonly options: TargetSpecificOptions | Nullish

// use only for tasks that cannot be executed in parallel (such as signing on windows and hdiutil on macOS due to file locking)
readonly buildQueueManager = new AsyncTaskManager(new CancellationToken())

protected constructor(
readonly name: string,
readonly isAsyncSupported: boolean = true
Expand All @@ -86,8 +89,8 @@ export abstract class Target {

abstract build(appOutDir: string, arch: Arch): Promise<any>

finishBuild(): Promise<any> {
return Promise.resolve()
async finishBuild(): Promise<any> {
await this.buildQueueManager.awaitTasks()
}
}

Expand Down
8 changes: 7 additions & 1 deletion packages/app-builder-lib/src/macPackager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,13 @@ export class MacPackager extends PlatformPackager<MacConfiguration> {
const artifactName = this.expandArtifactNamePattern(masOptions, "pkg", arch)
const artifactPath = path.join(outDir!, artifactName)
await this.doFlat(appPath, artifactPath, masInstallerIdentity, keychainFile)
await this.dispatchArtifactCreated(artifactPath, null, Arch.x64, this.computeSafeArtifactName(artifactName, "pkg", arch, true, this.platformSpecificBuildOptions.defaultArch))
await this.info.emitArtifactBuildCompleted({
file: artifactPath,
target: null,
arch: Arch.x64,
safeArtifactName: this.computeSafeArtifactName(artifactName, "pkg", arch, true, this.platformSpecificBuildOptions.defaultArch),
packager: this,
})
}

if (!isMas) {
Expand Down
31 changes: 30 additions & 1 deletion packages/app-builder-lib/src/packager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getArtifactArchName,
InvalidConfigurationError,
log,
MAX_FILE_REQUESTS,
orNullIfFileNotExist,
safeStringifyJson,
serializeToYaml,
Expand Down Expand Up @@ -41,6 +42,7 @@ import { resolveFunction } from "./util/resolve"
import { installOrRebuild, nodeGypRebuild } from "./util/yarn"
import { PACKAGE_VERSION } from "./version"
import { AsyncEventEmitter, HandlerType } from "./util/asyncEventEmitter"
import asyncPool from "tiny-async-pool"

async function createFrameworkInfo(configuration: Configuration, packager: Packager): Promise<Framework> {
let framework = configuration.framework
Expand Down Expand Up @@ -479,6 +481,18 @@ export class Packager {
const nameToTarget: Map<string, Target> = new Map()
platformToTarget.set(platform, nameToTarget)

let poolCount = Math.floor(packager.config.concurrency?.jobs || 1)
if (poolCount < 1) {
log.warn({ concurrency: poolCount }, "concurrency is invalid, overriding with job count: 1")
poolCount = 1
} else if (poolCount > MAX_FILE_REQUESTS) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can delete else

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, why?

Copy link
Contributor

@xyloflake xyloflake Mar 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, why?

Same question, I believe that is a necessary conditional

Copy link
Collaborator

@beyondkmp beyondkmp Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologized for the misunderstanding.
I mean

 if (poolCount < 1) {
        log.warn({ concurrency: poolCount }, "concurrency is invalid, overriding with job count: 1")
        poolCount = 1
  } 
 if (poolCount > MAX_FILE_REQUESTS) {}

However, this is just a coding convention, it's optional to change it.

log.warn(
{ concurrency: poolCount, MAX_FILE_REQUESTS },
`job concurrency is greater than recommended MAX_FILE_REQUESTS, this may lead to File Descriptor errors (too many files open). Proceed with caution (e.g. this is an experimental feature)`
)
}
const packPromises: Promise<any>[] = []

for (const [arch, targetNames] of computeArchToTargetNamesMap(archToType, packager, platform)) {
if (this.cancellationToken.cancelled) {
break
Expand All @@ -488,9 +502,21 @@ export class Packager {
const outDir = path.resolve(this.projectDir, packager.expandMacro(this.config.directories!.output!, Arch[arch]))
const targetList = createTargets(nameToTarget, targetNames.length === 0 ? packager.defaultTarget : targetNames, outDir, packager)
await createOutDirIfNeed(targetList, createdOutDirs)
await packager.pack(outDir, arch, targetList, taskManager)
const promise = packager.pack(outDir, arch, targetList, taskManager)
if (poolCount < 2) {
await promise
} else {
packPromises.push(promise)
}
}

await asyncPool(poolCount, packPromises, async it => {
if (this.cancellationToken.cancelled) {
return
}
await it
})

if (this.cancellationToken.cancelled) {
break
}
Expand All @@ -507,6 +533,9 @@ export class Packager {
await taskManager.awaitTasks()

for (const target of syncTargetsIfAny) {
if (this.cancellationToken.cancelled) {
break
}
await target.finishBuild()
}
return platformToTarget
Expand Down
10 changes: 0 additions & 10 deletions packages/app-builder-lib/src/platformPackager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,6 @@ export abstract class PlatformPackager<DC extends PlatformSpecificBuildOptions>
)
}

dispatchArtifactCreated(file: string, target: Target | null, arch: Arch | null, safeArtifactName?: string | null): Promise<void> {
return this.info.emitArtifactBuildCompleted({
file,
safeArtifactName,
target,
arch,
packager: this,
})
}

async pack(outDir: string, arch: Arch, targets: Array<Target>, taskManager: AsyncTaskManager): Promise<any> {
const appOutDir = this.computeAppOutDir(outDir, arch)
await this.doPack({
Expand Down
28 changes: 16 additions & 12 deletions packages/app-builder-lib/src/targets/AppxTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ const DEFAULT_RESOURCE_LANG = "en-US"
export default class AppXTarget extends Target {
readonly options: AppXOptions = deepAssign({}, this.packager.platformSpecificBuildOptions, this.packager.config.appx)

isAsyncSupported = false

constructor(
private readonly packager: WinPackager,
readonly outDir: string
Expand Down Expand Up @@ -145,18 +147,20 @@ export default class AppXTarget extends Target {
if (this.options.makeappxArgs != null) {
makeAppXArgs.push(...this.options.makeappxArgs)
}
await vm.exec(vm.toVmFile(path.join(vendorPath, "windows-10", signToolArch, "makeappx.exe")), makeAppXArgs)
await packager.sign(artifactPath)

await stageDir.cleanup()

await packager.info.emitArtifactBuildCompleted({
file: artifactPath,
packager,
arch,
safeArtifactName: packager.computeSafeArtifactName(artifactName, "appx"),
target: this,
isWriteUpdateInfo: this.options.electronUpdaterAware,
this.buildQueueManager.add(async () => {
await vm.exec(vm.toVmFile(path.join(vendorPath, "windows-10", signToolArch, "makeappx.exe")), makeAppXArgs)
await packager.sign(artifactPath)

await stageDir.cleanup()

await packager.info.emitArtifactBuildCompleted({
file: artifactPath,
packager,
arch,
safeArtifactName: packager.computeSafeArtifactName(artifactName, "appx"),
target: this,
isWriteUpdateInfo: this.options.electronUpdaterAware,
})
})
}

Expand Down
Loading