Skip to content

file_packager.py: Add option to embed file data in wasm binary #16050

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

Merged
merged 1 commit into from
Jan 22, 2022

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jan 17, 2022

This change not only adds the new option but uses this option
whenever file_packager is called from within emcc.

Very heavily inspired by the larger change in #16028.

Hopefully we can find a way deprecate and remove the old JS
embedding code since that seems strictly worse in every(?) way:

  • Larger code size (JS base64 encoding is larger than binary)
  • No possiblity of zero copy, memory-backed files
  • Less compatible with standalone wasm / WASI

@sbc100 sbc100 force-pushed the cleanup_file_packager2 branch from 92608cb to fc89621 Compare January 17, 2022 23:15
@sbc100 sbc100 force-pushed the file_packager_native branch from c4718b7 to eadafba Compare January 17, 2022 23:15
@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 17, 2022

@sbc100 sbc100 force-pushed the cleanup_file_packager2 branch from fc89621 to decb32d Compare January 17, 2022 23:22
@sbc100 sbc100 force-pushed the file_packager_native branch 3 times, most recently from 002aa28 to 9258fe2 Compare January 17, 2022 23:34
@sbc100 sbc100 force-pushed the file_packager_native branch from 9258fe2 to da737b9 Compare January 18, 2022 00:38
@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 18, 2022

Also depends on this llvm change: https://reviews.llvm.org/D117412

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Nice!

How much more do we need for this to work with WasmFS?

@kripken
Copy link
Member

kripken commented Jan 19, 2022

Perhaps mention the changelog?

Base automatically changed from cleanup_file_packager2 to main January 19, 2022 19:21
@sbc100 sbc100 force-pushed the file_packager_native branch 2 times, most recently from c3da14e to 2ecbd15 Compare January 19, 2022 21:26
@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 19, 2022

How much more do we need for this to work with WasmFS?

I think it should "just work" .. all it depends on is the FS_createDataFile function.

@sbc100 sbc100 enabled auto-merge (squash) January 19, 2022 21:28
@sbc100 sbc100 force-pushed the file_packager_native branch from 2ecbd15 to 93a23d6 Compare January 20, 2022 23:08
@sbc100 sbc100 disabled auto-merge January 20, 2022 23:23
This change not only adds the new option but uses this option
whenever file_packager is used from within emcc.

Hopefully we can find a way deprecate and remove the old JS
embedded since that seems strictly worse in almost ever way.

- Larger code size (JS base64 encoding is larger than binary)
- No possiblity of zero copy, memory-backed files
- Less compatible with standalone wasm / WASI
@sbc100 sbc100 force-pushed the file_packager_native branch from 93a23d6 to adbf8dd Compare January 21, 2022 23:05
@sbc100 sbc100 enabled auto-merge (squash) January 21, 2022 23:05
@sbc100 sbc100 merged commit fb0d9ab into main Jan 22, 2022
@sbc100 sbc100 deleted the file_packager_native branch January 22, 2022 01:18
@kripken
Copy link
Member

kripken commented Jan 23, 2022

kripken added a commit that referenced this pull request Jan 25, 2022
This practically worked since #16050, but the JS side needs to support a
missing parent, which is how the file packager emits it.

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

Successfully merging this pull request may close these issues.

2 participants