Use base64 encoding in embedded files in file packager

[crudelios]

Hi,

I'm attempting to creating an emscripten build of a game. I want to embeed the game's assets to the html file as I want everything to load from a single file.

However, when I try to embed the assets directory (which contains about 1.000 files totalling 15 MB), the script uses an awful lot of memory and eventually fails with Allocation failed - JavaScript heap out of memory. This happens even if I set node.js's max memory size to 8GB.

Using --preload-file works fine but I do want to add the assets to the html file.

I'm building with cmake using the following flags:

set_target_properties(${SHORT_NAME} PROPERTIES LINK_FLAGS "-lidbfs.js -s SINGLE_FILE=1 -s DEMANGLE_SUPPORT=1 -s DYNCALLS=1 --shell-file ${PROJECT_SOURCE_DIR}/res/shell.html --embed-file ${PROJECT_SOURCE_DIR}/assets@/assets -s INITIAL_MEMORY=268435456 -s ALLOW_MEMORY_GROWTH=1 -s DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=0 -s SAFE_HEAP=1 -s INVOKE_RUN=0 -s EXPORTED_FUNCTIONS=[\"_main\"] -s EXPORTED_RUNTIME_METHODS=[\"callMain\",\"FS\"]  --bind")

An example build with the failure is available here: https://github.com/crudelios/julius/runs/2858867432

Thanks for your help!

[kripken]

I think this is because JS has no efficient way to embed binary data in it. So the way we embed files can use a lot of memory (by the JS VM as it parses it). Embedding files is nice sometimes for simplicity, but for anything large like your 1,000 files of 15MB, preloading or another solution is going to be better.

[crudelios]

Thanks for the feedback. It makes sense that Javascript is not good with binary files.

However, I still find it strange that the compiler manages to create a 4mb html file with the entire game executable using only about 150mb of ram then for 15mb of assets it uses over 8gb.

For reference, the memory problem seems to happen on the acorn optimizer.

[kripken]

I may have misunderstood before - is this failing during compilation (in acorn as the last comment mentions) or at runtime ("the script" in the first comment refers to the output script, I assumed)?

If it's at compile time, then I'd guess this is hitting a bad case in the acorn AST, like maybe quadratic memory is used to construct something.

[crudelios]

Yeah, it fails during compilation. Using --preload-assets compiles fine though.

The specific error message is this one:

emcc: error: '/home/runner/work/julius/julius/emsdk/node/14.15.5_64bit/bin/node /home/runner/work/julius/julius/emsdk/upstream/emscripten/tools/acorn-optimizer.js /tmp/emscripten_temp_1mfik021/augustus.js.pp.js safeHeap' failed (received SIGABRT (-6))

And it fails with the following error:

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

[kripken]

I see. In that case, a way to consider improving this would be to look into the JS we emit in this case, and why acorn does so badly at it. The JS is emitted by tools/file_packager.py. Perhaps for example acorn does badly with very very long array literals or something like that.

[crudelios]

Using file_packager.py to create a js with the data does create a 72 mb file.

Unlike the executable js, which is compressed to a string (I'm guessing a base64-encoded version of the binary?), the js file with the data stores the binary files using a rather space inefficient int array.

That may be causing the problems with acorn.

[kripken]

I think we did optimize the executable at some point to base64, yes. It does seem likely that could help here. Renaming this issue for that.

[crudelios]

I can add the PR myself as it seems simple enough to implement. In fact I already implemented base64 encoding to the file_manager.py and the generated JS data file dropped from 62mb to 18mb.

My only question is, should the base64 always be used or should there be an option? I don't really see any advantage in keeping the current int array even as an option, but I don't really know enough about this project to say what's best or not.

[kripken]

A PR would be great!

I agree - I don't see a reason to keep the current int array option at all. Let's just replace it. Thanks @crudelios !