Skip to content

Make file packager --no-heap-copy always apply, and remove the option #12027

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 4 commits into from
Aug 26, 2020

Conversation

kripken
Copy link
Member

@kripken kripken commented Aug 25, 2020

The old default was to copy a file package into main memory (into space
malloc'd for it). That was not memory-efficient as it meant the main memory
needed to be bigger, and also the memory could never be freed. While
mmap on that could be fast, we added the --no-heap-copy option to
keep file data on the JS heap and not copy it in. That uses less memory
but still is just as fast at fread etc.

This makes that newer option the default, and removes the option to do it
the old way. This is better for memory usage, and in any case people using
file data in a heavy way may be rolling their own solutions anyhow.

This also removes the file system usage of getMemory(), which would
perform allocation during startup from JS, which is a source of complexity
I am working to remove for WebAssembly/binaryen#3043

@kripken kripken requested review from juj and sbc100 August 25, 2020 00:00
@@ -2435,7 +2434,7 @@ def test_preload_module(self):

def test_mmap_file(self):
create_test_file('data.dat', 'data from the file ' + ('.' * 9000))
for extra_args in [[], ['--no-heap-copy']]:
for extra_args in [[]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove these this loop, and below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks.

@kripken kripken merged commit 6fa74b3 into master Aug 26, 2020
@kripken kripken deleted the no-noheapcopy branch August 26, 2020 18:48
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