Great suggestion. We also previously patched the build process to enable parallel compilation. However, on Windows, signing would often fail due to file locks, causing the entire build to fail. We later switched to running multiple Docker containers for parallel compilation.

If we can resolve the issue of file locks during signing, it should be possible to implement concurrent compilation.

We also previously patched the build process to enable parallel compilation.

Do you mind explaining this or reference a relevant commit so I can take a look?

If we can resolve the issue of file locks during signing, it should be possible to implement concurrent compilation.
Thank you, I believe if we do have problems with windows, we should start out with linux and slowly work the way up with supporting all platforms as we understand the challenges introduced. I am mostly worried about race conditions (such as reading the unpacked folder at the same time could result in EBUSY and could fail the build if not handled correctly).

I will think about that to see if I can come up with something.

@beyondkmp do you think the builds could be temporarily placed in specific folders and the lockfile could be cloned for each one of the builds and then we execute signing, which can use their specific lockfiles?

@mmaietta would love it if you got some suggestions

I don't think this can blankly be supported, as some programs lock usage during execution. I'm thinking hdiutil, some Windows targets that require building in sync (msi-wrapped IIRC), as well as the file locks that @beyondkmp mentioned (such as signtool and azure signing). There's too many edge cases that'll require complex logic to work around. I'll investigate further, but it's fairly low priority on my todo list as I envision this would require a significant refactor.

it should be possible to implement concurrent compilation.

Also @beyondkmp, this is incorrect. Please refer to electron-builder as a packager, not a compiler (or bundler), as it historically has caused significant confusion in expectations by the end-user/dev.

We also previously patched the build process to enable parallel compilation.
We later switched to running multiple Docker containers for parallel compilation.

I also don't know what this is referring to haha, can you please elaborate?

I don't think this can blankly be supported, as some programs lock usage during execution. I'm thinking hdiutil, some Windows targets that require building in sync (msi-wrapped IIRC), as well as the file locks that @beyondkmp mentioned (such as signtool and azure signing). There's too many edge cases that'll require complex logic to work around. I'll investigate further, but it's fairly low priority on my todo list as I envision this would require a significant refactor.

I completely agree with it being a significant refactor. Why can't we wait for some tasks to finish concurrently and after waiting for them, when they finish, execute the tasks sequentially that need sync?

I believe more than 95% of the packaging can be concurrent but I am pretty sure it is gonna be really big refactor.

Just to help me get started on brainstorming a refactor, what areas are you aware of that can be concurrent?

My biggest fear here are edge cases / race conditions / file locks that will make it super difficult to debug end-user/dev issue reports.

Just to help me get started on brainstorming a refactor, what areas are you aware of that can be concurrent?

I'll try to make up a list and make some efforts in a fork I create so that I can tell what parts could be concurrent without raising race conditions.

My biggest fear here are edge cases / race conditions / file locks that will make it super difficult to debug end-user/dev issue reports.

I don't think debugging will be difficult but handling the race conditions would be pretty much the most challenging part. Especially since JavaScript/TypeScript ecosystem has near zero atomics support. This would actually need refactors to the core architecture but I'm pretty sure what we'd end up with is a much more optimized packager.

I think most of the concurrent logic needs to be written out of js/ts itself if it makes sense. Although if we do get asynchronous stuff working correctly, we can do this in js land I suppose.

See #8405 (relevant)

needs to be written out of js/ts itself

As in migrate to Go or another platform-agnostic binary?

As in migrate to Go or another platform-agnostic binary?

Yes. But not migrate, just write some part in Go that we can call from JS through IPC or something similar. Something like we had with app-builder-bin I suppose.

Hmmmm. We're ironically trying to move away from app-builder-bin to JS so that it's easier to maintain the codebase and make modifications (example: node module collector to support additional package managers with hoisting) that we weren't able to make to app-builder-bin. Same thing goes for electron artifact downloading (migrating to the official @electron/get)

@mmaietta I get it but you also need to understand that JS/TS is not designed for concurrent operations. Of course, you could always use web workers but the overhead of spawning a web worker will make it significantly slower. So I am really skeptical of this working well on JS but I'll give it a spin!

I know that and acknowledge that.

As a test run, would you be willing to try this patch-package out with your project? This basically allows 8 (MAX_FILE_REQUESTS) Targets to be processed concurrently. Seemed to work fine on my test project, but I didn't test signing or dmg creation specifically (albeit, concurrent unit tests show that those do lock files).
You can adjust builder_util_1.MAX_FILE_REQUESTS to a concurrency number of your choosing

app-builder-lib+26.0.9.patch

diff --git a/node_modules/app-builder-lib/out/packager.js b/node_modules/app-builder-lib/out/packager.js
index 81c1f08..79ece76 100644
--- a/node_modules/app-builder-lib/out/packager.js
+++ b/node_modules/app-builder-lib/out/packager.js
@@ -24,6 +24,7 @@ const resolve_1 = require("./util/resolve");
 const yarn_1 = require("./util/yarn");
 const version_1 = require("./version");
 const asyncEventEmitter_1 = require("./util/asyncEventEmitter");
+const tiny_async_pool_1 = require("tiny-async-pool");
 async function createFrameworkInfo(configuration, packager) {
     let framework = configuration.framework;
     if (framework != null) {
@@ -353,6 +354,7 @@ class Packager {
             const packager = await this.createHelper(platform);
             const nameToTarget = new Map();
             platformToTarget.set(platform, nameToTarget);
+            const packPromises = [];
             for (const [arch, targetNames] of (0, targetFactory_1.computeArchToTargetNamesMap)(archToType, packager, platform)) {
                 if (this.cancellationToken.cancelled) {
                     break;
@@ -361,8 +363,9 @@ class Packager {
                 const outDir = path.resolve(this.projectDir, packager.expandMacro(this.config.directories.output, builder_util_1.Arch[arch]));
                 const targetList = (0, targetFactory_1.createTargets)(nameToTarget, targetNames.length === 0 ? packager.defaultTarget : targetNames, outDir, packager);
                 await createOutDirIfNeed(targetList, createdOutDirs);
-                await packager.pack(outDir, arch, targetList, taskManager);
+                packPromises.push(packager.pack(outDir, arch, targetList, taskManager));
             }
+            await (0, tiny_async_pool_1.default)(builder_util_1.MAX_FILE_REQUESTS, packPromises, it => it);
             if (this.cancellationToken.cancelled) {
                 break;
             }

@staniel359 I'll try this with my fork of muffon.

@mmaietta I'll give it a try soon enough but I'm a little busy with some stuff right now :)

So I've spent some significant time researching this further, and overall, with minor modifications, it works really well and REALLY fast.

The downside that currently has me blocked is that the artifact publishing logic and update info generator are keyed off of the order of events it receives, which normally is the exact order of the arch(s) configured to build. When running in an async pool, it loses that ordering as some artifacts unsurprisingly finish faster (such as arm64 .app being compiled faster than the x64 version, which in this case, causes the UpdateInfo to have primary file property keyed to arm64 instead of intended x64)

In the interim, to test progress, I did write a test suite for concurrent builds in #8920

But I can't merge that test suite currently since it increases the duration of the CI suite by 2-3x. When running with my sync pool implementation, the impact on CI duration is pretty much not noticeable from initial tests. Just need to fix the publishing-ordering logic first

So I've spent some significant time researching this further, and overall, with minor modifications, it works really well and REALLY fast.

Great to hear that!

The downside that currently has me blocked is that the artifact publishing logic and update info generator are keyed off of the order of events it receives, which normally is the exact order of the arch(s) configured to build. When running in an async pool, it loses that ordering as some artifacts unsurprisingly finish faster (such as arm64 .app being compiled faster than the x64 version, which in this case, causes the UpdateInfo to have primary file property keyed to arm64 instead of intended x64)

That's just reordering the publishing to be in the end after all builds have completed, am I right?

In the interim, to test progress, I did write a test suite for concurrent builds in #8920

Looks great!

But I can't merge that test suite currently since it increases the duration of the CI suite by 2-3x. When running with my sync pool implementation, the impact on CI duration is pretty much not noticeable from initial tests. Just need to fix the publishing-ordering logic first

Understandable!

I appreciate your efforts mike! Thank you for this!

That's just reordering the publishing to be in the end after all builds have completed, am I right?

Yes, but it's not just a return of the promises. It's a generic ArtifactCreated event listener, so I'm trying to figure out how to pass in the order of operations (Arch order) that were originally being achieved.