feat: merge ASARs #34

indutny-signal · 2021-12-16T16:22:18Z

In short, this adds a new option fuseASARs that, instead of checking that ASARs are exactly the same, will attempt to fuse them together into one and run lipo on shared bindings.

uniqueAllowList` option handles files that are unique to each ASAR. Ultimately the goal is to put all of them together in the final ASAR, but I decided to go with an explicit allowlist so that the end user will always be aware of the extra files.

This helped us reduce universal .dmg size from 300mb to 240mb because we have quite a lot of data in our asar file at Signal.

Example of debug output

  electron-universal fusing x64 and arm64 asars +16ms
  electron-universal Fusing /var/folders/9f/5h519zwn7kz7rbwr_wr7sptm0000gn/T/electron-universal-VDiwOl/Tmp.app/Contents/Resources/app.asar and /Users/indutny/Code/signalapp/Signal-Desktop-Private/release/mac-universal--arm64/Signal.app/Contents/Resources/app.asar +1ms
  electron-universal Extracting /var/folders/9f/5h519zwn7kz7rbwr_wr7sptm0000gn/T/electron-universal-VDiwOl/Tmp.app/Contents/Resources/app.asar to /var/folders/9f/5h519zwn7kz7rbwr_wr7sptm0000gn/T/x64-Vfv57T +985ms
  electron-universal Extracting /Users/indutny/Code/signalapp/Signal-Desktop-Private/release/mac-universal--arm64/Signal.app/Contents/Resources/app.asar to /var/folders/9f/5h519zwn7kz7rbwr_wr7sptm0000gn/T/arm64-imyBcQ +2s
  electron-universal Creating unique directory: node_modules/@signalapp/signal-client/prebuilds/darwin-arm64 +2s
  electron-universal Copying unique file: node_modules/@signalapp/signal-client/prebuilds/darwin-arm64/node.napi.node +1ms
  electron-universal Copying unique file: node_modules/ringrtc/build/darwin/libringrtc-arm64.node +6ms
  electron-universal Creating unique directory: node_modules/sharp/vendor/8.11.3/darwin-arm64v8 +12ms
  electron-universal Copying unique file: node_modules/sharp/vendor/8.11.3/darwin-arm64v8/platform.json +0ms
  electron-universal Copying unique file: node_modules/sharp/vendor/8.11.3/darwin-arm64v8/versions.json +2ms
  electron-universal Creating unique directory: node_modules/sharp/vendor/8.11.3/darwin-arm64v8/lib +0ms
  electron-universal Copying unique file: node_modules/sharp/vendor/8.11.3/darwin-arm64v8/lib/libvips-cpp.42.dylib +0ms
  electron-universal Copying unique file: node_modules/sharp/build/Release/sharp-darwin-arm64v8.node +22ms
  electron-universal Fusing binding: node_modules/better-sqlite3/build/Release/better_sqlite3.node +3ms
  electron-universal Creating archive at /var/folders/9f/5h519zwn7kz7rbwr_wr7sptm0000gn/T/electron-universal-VDiwOl/Tmp.app/Contents/Resources/app.asar +28ms
  electron-universal Done fusing +11s
  electron-universal copying snapshot file Contents/Frameworks/Electron Framework.framework/Versions/A/Resources/v8_context_snapshot.arm64.bin to target application +2s
  electron-universal moving final universal app to target destination +4ms

indutny-signal · 2021-12-16T22:53:13Z

src/asar-utils.ts

+  0xcffaedfe,
+
+  // Universal
+  0xcafebabe,


I believe there's a slight risk of misidentifying Java bytecode files as macho, but in that case lipo will fail anyway so maybe we shouldn't bother?

MarshallOfSound · 2022-01-10T20:40:31Z

src/asar-utils.ts

+  x64,
+  arm64,
+  output,


Can these all be more specific, I assume they're paths to asar files?

So x64AsarPath, arm64AsarPath, outputAsarPath, etc.

MarshallOfSound · 2022-01-10T20:40:58Z

src/asar-utils.ts

+  x64,
+  arm64,
+  output,
+  lipo = 'lipo',


Is this the path / name of the lipo binary? If so let's go with lipoPath, more indicative

Also why is this an option, it isn't a user-facing option and afaics we don't do anything similar elsewhere

Agreed. Let's drop this.

MarshallOfSound · 2022-01-10T20:45:33Z

src/index.ts

+  /**
+   * Fuse x64 and arm64 ASARs into one.
+   */
+  fuseASARs?: boolean;
+  /**
+   * Minimatch pattern of paths that are allowed to be unique in ASARs.
+   */
+  fuseASARsAllowList?: string;


What happens if they're on this list, does it take the x64 file? the arm64 file? etc.

I think it would be better to somehow allow folks to choose, or preferably not allow this at all. It leads to uncertainty, you should just form your ASAR file correctly.

What happens if they're on this list, does it take the x64 file? the arm64 file? etc.

It takes either x64 or arm64 file that is not present in the other platform's asar.

I think it would be better to somehow allow folks to choose, or preferably not allow this at all. It leads to uncertainty, you should just form your ASAR file correctly.

The "unique" files in this jargon will likely have platform specific name (e.g. node_modules/sharp/build/${platform}-${arch}, at least this is what we see at Signal Desktop), and so I believe that this option is usable in the current form. The motivation for introducing it is that I wanted users to have complete control over which files are going to be introduced into ASAR. My worry is that there might be a package that won't play well with such merging scheme and users won't know about it until it will start crashing in production.

Ah I see, so these files to be clear exist in one asar but do not exist in the other? 🤔 I don't know what a better name is but it's definitely not clear what that does if I didn't grok it first try.

Maybe singleArchFiles or something idk, open to ideas here.

Ah I see, so these files to be clear exist in one asar but do not exist in the other? 🤔 I don't know what a better name is but it's definitely not clear what that does if I didn't grok it first try.

You got it right.

Maybe singleArchFiles or something idk, open to ideas here.

singleArchFiles certainly sounds better than what I came up with, let me change it to this. I'm out of ideas, otherwise!

MarshallOfSound · 2022-01-10T20:46:17Z

src/index.ts

+  /**
+   * Fuse x64 and arm64 ASARs into one.
+   */
+  fuseASARs?: boolean;


Can we not use the word "fuse" here as fuses are a different Electron concept.

Either mergeASARs or lipoNativeModules might be better options here. The latter definition if we remove the allow list option below because then that's all this option does.

Renamed to mergeASARs, which is frankly what I came up originally 😂

MarshallOfSound · 2022-01-10T20:48:02Z

src/asar-utils.ts

+      const destination = await fs.realpath(path.resolve(x64Dir, binding));
+
+      d(`fusing binding: ${binding}`);
+      execFileSync(lipo, [source, destination, '-create', '-output', destination]);


Because of the way MACHO_MAGIC works couldn't this be trying to fuse universal/x64 or x64 and x64 that don't match? Not familiar enough with lipo to know what happens when you try to merge things like that.

I think I should just drop the Universal constant from MACHO_MAGIC. The merging of x64 and x64 would fail AFAIK and it should be fine because the binding binaries shouldn't be different unless they were compiled for different platforms!

indutny-signal · 2022-01-10T22:49:49Z

Thanks for looking into this @MarshallOfSound . I've addressed your feedback with the latest push.

indutny-signal · 2022-01-11T22:49:14Z

Realized that I forgot to drop Universal signatures from MACHO_MAGIC. Should be ready for another look if you have time. Thanks!

indutny-signal · 2022-01-20T22:11:02Z

Friendly ping @MarshallOfSound

electron-bot · 2022-01-24T21:42:17Z

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

indutny-signal · 2022-01-24T21:55:33Z

Yay, thank you so much!

indutny-signal force-pushed the feature/fuse-asars branch from d11807e to 3cfc895 Compare December 16, 2021 16:23

erickzhao requested review from a team and MarshallOfSound December 16, 2021 18:29

feat: fuse ASARs

2bcde3c

indutny-signal force-pushed the feature/fuse-asars branch from 3cfc895 to 2bcde3c Compare December 16, 2021 22:45

indutny-signal commented Dec 16, 2021

View reviewed changes

jakobvogel mentioned this pull request Dec 22, 2021

Resources Not Found when Using Native Dependencies #35

Closed

MarshallOfSound reviewed Jan 10, 2022

View reviewed changes

Rename, improve

01d05c5

indutny-signal added 2 commits January 10, 2022 15:02

Rename option

bd9e1f3

Drop universal from MACHO_MAGIC

a99248e

indutny-signal changed the title ~~feat: fuse ASARs~~ feat: merge ASARs Jan 20, 2022

MarshallOfSound approved these changes Jan 24, 2022

View reviewed changes

MarshallOfSound merged commit 38ab1c3 into electron:master Jan 24, 2022

electron-bot added the released label Jan 24, 2022

indutny-signal deleted the feature/fuse-asars branch January 24, 2022 21:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: merge ASARs #34

feat: merge ASARs #34

indutny-signal commented Dec 16, 2021 •

edited

Loading

indutny-signal Dec 16, 2021

MarshallOfSound Jan 10, 2022

indutny-signal Jan 10, 2022

MarshallOfSound Jan 10, 2022

MarshallOfSound Jan 10, 2022

indutny-signal Jan 10, 2022

MarshallOfSound Jan 10, 2022

indutny-signal Jan 10, 2022

MarshallOfSound Jan 10, 2022

indutny-signal Jan 10, 2022

indutny-signal Jan 10, 2022

MarshallOfSound Jan 10, 2022

indutny-signal Jan 10, 2022

MarshallOfSound Jan 10, 2022

indutny-signal Jan 10, 2022

indutny-signal commented Jan 10, 2022

indutny-signal commented Jan 11, 2022

indutny-signal commented Jan 20, 2022

electron-bot commented Jan 24, 2022

indutny-signal commented Jan 24, 2022

feat: merge ASARs #34

feat: merge ASARs #34

Conversation

indutny-signal commented Dec 16, 2021 • edited Loading

Example of debug output

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

indutny-signal commented Jan 10, 2022

indutny-signal commented Jan 11, 2022

indutny-signal commented Jan 20, 2022

electron-bot commented Jan 24, 2022

indutny-signal commented Jan 24, 2022

indutny-signal commented Dec 16, 2021 •

edited

Loading