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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 22, 2022
Merged

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

merged 1 commit into from
Jan 22, 2022

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jan 17, 2022 •
edited
Loading

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

This change depends of a chain of cleanup/refactoring PRs than need to land first:

@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 mentioned this pull request Jan 17, 2022
Closed
@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

@sbc100 sbc100 force-pushed the cleanup_file_packager2 branch 2 times, most recently from 52ff804 to 3dbf141 Compare January 19, 2022 00:02
@sbc100 sbc100 force-pushed the file_packager_native branch from da737b9 to d33e5dd Compare January 19, 2022 00:08
@sbc100 sbc100 mentioned this pull request Jan 19, 2022
Merged
@sbc100 sbc100 force-pushed the file_packager_native branch from d33e5dd to 2bc217b Compare January 19, 2022 00:42
@sbc100 sbc100 requested a review from kripken January 19, 2022 00:43
This was referenced Jan 19, 2022
Merged
Merged
kripken
kripken approved these changes Jan 19, 2022
View reviewed changes
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
@sbc100
file_packager.py: Add option to embed file data in wasm binary
adbf8dd 
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

This seems to have broken the roller on windows:

https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8824247839692786161/+/u/Emscripten_testsuite__upstream__other_/stdout

sbc100 added a commit that referenced this pull request Jan 23, 2022
@sbc100
Fix file embedding tests under windows that were broken by #16050
7fb8297
sbc100 added a commit that referenced this pull request Jan 23, 2022
@sbc100
Fix file embedding tests under windows that were broken by #16050
036c822
sbc100 added a commit that referenced this pull request Jan 23, 2022
@sbc100
Fix file embedding tests under windows that were broken by #16050
7b28b75
sbc100 added a commit that referenced this pull request Jan 23, 2022
@sbc100
Fix file embedding tests under windows that were broken by #16050
e94a883
sbc100 added a commit that referenced this pull request Jan 23, 2022
@sbc100
Fix file embedding tests under windows that were broken by #16050 (#1…
6531264 
…6102)
@kripken kripken mentioned this pull request Jan 25, 2022
Merged
kripken added a commit that referenced this pull request Jan 25, 2022
@kripken
[WasmFS] Support file embedding (#16105)
611b14a 
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
sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 25, 2025
@sbc100
[file-packager] Drop support for --embed with JS output
6729852 
The object file output is more efficient way of embedding and has been
recommended via a warning since emscripten-core#16050 (early 2022).

With this simplification the file packager now has essentially two
output modes:

1. Preloading via JS
2. Embedding via and object file
@sbc100 sbc100 mentioned this pull request Aug 25, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kripken kripken kripken approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sbc100 @kripken