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

fix: order files in asar for incremental updates #8128

Merged

Conversation

indutny-signal
Copy link
Contributor

@indutny-signal indutny-signal commented Mar 12, 2024

ASAR file begins with a header that list all files and an offset to each file in the rest of the file. When a file placed early in ASAR changes its length - it means that all subsequent file declarations in the header will have their offsets updated. While harmless by itself, this negatively affects the incremental download size as more of the installer binary is different from what it used to be.

In this change we order files in asar such that:

  • Dependencies/node_modules come first (they change least often)
  • Main app files come last (they change more frequently)

Additionally, files in asar are now ordered alphabetically within each fileset to guarantee stable output.

All of above results in 2x improvement of incremental download size for updates that don't change dependencies.

ASAR file begins with a header that list all files and an offset to each
file in the rest of the file. When a file placed early in ASAR changes
its length - it means that all subsequent file declarations in the
header will have their offsets updated. While harmless by itself, this
negatively affects the incremental download size as more of the
installer binary is different from what it used to be.

In this change we order files in asar such that:

- Dependencies/node_modules come first (they change least often)
- Main app files come last (they change more frequently)

Additionally, files in asar are now ordered alphabetically within each
fileset to guarantee stable output.

All of above results in 2x improvement of incremental download size.
Copy link

changeset-bot bot commented Mar 12, 2024

🦋 Changeset detected

Latest commit: 8a13d81

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
app-builder-lib Patch
dmg-builder Patch
electron-builder-squirrel-windows Patch
electron-builder Patch
electron-forge-maker-appimage Patch
electron-forge-maker-nsis-web Patch
electron-forge-maker-nsis Patch
electron-forge-maker-snap Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Mar 12, 2024

Deploy Preview for car-park-attendant-cleat-11576 ready!

Name Link
🔨 Latest commit 8a13d81
🔍 Latest deploy log https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/65f08aec882ec90008e42c1d
😎 Deploy Preview https://deploy-preview-8128--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@beyondkmp
Copy link
Contributor

This is so cool.

@indutny-signal
Copy link
Contributor Author

Now that I think more about it - is there any point of having separate file sets? Could we merge them all together without a risk?

@beyondkmp
Copy link
Contributor

@indutny-signal I don't quite understand; could you explain in more detail? Does this involve merging all files together?

@indutny-signal
Copy link
Contributor Author

@beyondkmp no, only merging the file sets. The resulting ASAR should be the same if maybe ordered a bit more consistently.

@mmaietta
Copy link
Collaborator

Nice work!

Related note re: the filesets and merging them, I've been yet to figure out their true purpose for being separated. I've been working in my off time to migrate us to the official electron/asar, but still not to any success yet due to the way we handle filesets

@indutny-signal
Copy link
Contributor Author

@mmaietta thanks!

Yeah, it is... funky. I think we have separate file sets for separate matchers because matchers can specify "from" and "to" paths, but I'm not entirely sure why these have to be separate. If we ever migrate to electron/asar - that I mean that I should send a PR to them as well 😂

@indutny-signal
Copy link
Contributor Author

indutny-signal commented Mar 12, 2024

Blerg, sorry for snapshots failing. It makes sense that all offsets are changed, but I couldn't get tests running properly on my arm64 macbook. FWIW, I tested this by packaging our app which has ASAR integrity checks on, and verifying that it starts and runs without issues.

@mmaietta
Copy link
Collaborator

@indutny-signal let me take a shot at updating them from my side on my arm64 mac
cmd to run is UPDATE_SNAPSHOT=true TEST_FILES=HoistedNodeModuleTest pnpm -w run test-linux

@indutny-signal
Copy link
Contributor Author

@mmaietta ah, that's what I was missing. Let me run it real quick. I actually have docker installed now.

@indutny-signal
Copy link
Contributor Author

@mmaietta done

@indutny-signal
Copy link
Contributor Author

I'm actually not entirely sure why it is failing in CI. It seems to pass locally...

@mmaietta
Copy link
Collaborator

mmaietta commented Mar 12, 2024

Will make this into the next release once I can get all snapshots updated, I'll take a look!

Related note re: differential updates, MacOS differential is now supported in electron-updater@6.3.0-alpha.1. As with all electron-updater changes, please make sure to thoroughly test from your side

@mmaietta mmaietta merged commit 555dc90 into electron-userland:master Mar 12, 2024
13 checks passed
@indutny-signal indutny-signal deleted the feature/order-asar-files branch March 12, 2024 17:30
@indutny-signal
Copy link
Contributor Author

Aww, thanks so much!

We use our own differential update system so we won't get a chance to test it :(

@mmaietta
Copy link
Collaborator

Very cool! Out of curiosity, does your differential update system work with dmgs?

@indutny-signal
Copy link
Contributor Author

@mmaietta nope, we use zip files!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants