suggestion: parallel builds

[xyloflake]

So recently, while working on ci, I had to configure electron builder to build for both x64 and aarch64/arm64.

I was pretty frustrated by the build times (we have to build for a lot of linux platforms + 2 architectures) which was around 16 minutes. I tried something to reduce it. I broke down the 2 architecture into 2 commands (for linux only):

1. release normally (x86_64)

electron-builder --publish always

2. release for aarch64/arm64

electron-builder --publish always --arm64

I then used bash to execute both simultaneously

electron-builder --publish always & \
electron-builder --publish always --arm64 & \
wait

This executed both at the same time and the total time was now down to 9 minutes from 16.

What this experiment has proven is that electron builder could explore the possibilities of building binaries as parallel jobs, which may speed up build times exponentially depending on cpu.

That about configuration on number of parallel jobs, by default I suggest setting it to NUM_CPU_CORES and if required, modify through electron-builder.json through some property like

{
  "parallelJobs": 4
}

I've also thought it out and figured out a possibility of race conditions while publishing builds to github (or similar service). For this I suggest waiting for all builds to finish and then uploading all the assets at once in one single job.

[beyondkmp]

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.

[xyloflake]

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.

[xyloflake]

@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?

[xyloflake]

@mmaietta would love it if you got some suggestions

[mmaietta]

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?

[xyloflake]

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.

[mmaietta]

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.

[xyloflake]

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.

[xyloflake]

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)

[mmaietta]

needs to be written out of js/ts itself

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

[xyloflake]

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.

[mmaietta]

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)

[xyloflake]

@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!

[mmaietta]

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;
             }

[xyloflake]

@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 :)