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

--separate-asm + -s BINARYEN=1 still downloads .asm.js code #4909

Closed
juj opened this issue Feb 1, 2017 · 6 comments
Closed

--separate-asm + -s BINARYEN=1 still downloads .asm.js code #4909

juj opened this issue Feb 1, 2017 · 6 comments
Labels

Comments

@juj
Copy link
Collaborator

juj commented Feb 1, 2017

Running em++ tests\hello_world.cpp --separate-asm -s BINARYEN=1 -o a.html, gives the following in the generated a.html file:

      var xhr = new XMLHttpRequest();
      xhr.open('GET', 'a.wasm', true);
      xhr.responseType = 'arraybuffer';
      xhr.onload = function() {
        Module.wasmBinary = xhr.response;

var script = document.createElement('script');
script.src = "a.asm.js";
script.onload = function() {
  setTimeout(function() {
    
      var script = document.createElement('script');
      script.src = "a.js";
      document.body.appendChild(script);

  }, 1); // delaying even 1ms is enough to allow compilation memory to be reclaimed
};
document.body.appendChild(script);

      };
      xhr.send(null);

which downloads .asm.js even when generating .asm.js was not requested. The extra code here is

      var xhr = new XMLHttpRequest();
      xhr.open('GET', 'a.wasm', true);
      xhr.responseType = 'arraybuffer';
      xhr.onload = function() {
        Module.wasmBinary = xhr.response;

//var script = document.createElement('script');
//script.src = "a.asm.js";
//script.onload = function() {
//  setTimeout(function() {
    
      var script = document.createElement('script');
      script.src = "a.js";
      document.body.appendChild(script);

//  }, 1); // delaying even 1ms is enough to allow compilation memory to be reclaimed
//};
//document.body.appendChild(script);

      };
      xhr.send(null);

The solution is to drop the --separate-asm flag here, which removes the extra code.

This hints that perhaps we should make --separate-asm flag be ignored when targeting only Wasm, since it generates this redundant code?

@juj
Copy link
Collaborator Author

juj commented Feb 1, 2017

Related, building with -s BINARYEN_METHOD='native-wasm,asmjs' implies --separate-asm, since 'native-wasm,asmjs' architecture requires that .asm.js file is stored separately.

However if one does pass --separate-asm explicitly with -s BINARYEN_METHOD='native-wasm,asmjs', it makes things worse by unconditionally downloading both files, like shown above. So I suppose when -s BINARYEN=1, we should silently ignore --separate-asm.

@kripken
Copy link
Member

kripken commented Feb 1, 2017

Well, if the asm.js might be used - like when the method is native-wasm,asmjs - then we do need to load the asm.js. So unconditionally preloading both wasm and asm.js in that case is right, I think.

Maybe we could just add a warning if --separate-asm is passed but the asm.js is not going to be used (not in the METHOD)? And if we don't already, document that --separate-asm will load the asm.js for you (when generating html).

juj added a commit to juj/emscripten that referenced this issue Feb 1, 2017
… code when --separate-asm and -s BINARYEN=1 are passed.
@juj
Copy link
Collaborator Author

juj commented Feb 1, 2017

I think we should never unconditionally download both, but first figure out which one would be needed, and only download that one? The above PR does seem to work well at in at least avoiding the .asm.js download at startup, does that make sense to you? Although I wonder if I'm seeing this right and it's now doing a sync XHR to obtain it when wasm support is disabled, hmm.

@kripken
Copy link
Member

kripken commented Feb 1, 2017

Well, that's assuming we can never be surprised later. E.g. you might see the browser has wasm support, but when you try to instantiate the wasm, it might fail for some reason. I guess the question is whether we want to support that. Currently, binaryen tries each method and checks for failure, it doesn't assume it can know ahead of time which will work (if it could know that, it wouldn't need to check one by one).

How important is it to optimize the case of fallback/multiple methods? Any build with both asm and wasm enabled will be a poorly-optimized one anyhow.

@juj
Copy link
Collaborator Author

juj commented Feb 14, 2017

Ideally we would in such a scenario first download .wasm, try it, if it doesn't work, only then download .asm.js.

Supporting fallback is important, at least for a while, but at present the -s BINARYEN_METHOD=native-wasm,asmjs is kind of unusable because of the asm.js coming out slow, that we ask people to build twice and manually develop the .html file to do the fallback they want.

juj added a commit to juj/emscripten that referenced this issue Feb 14, 2017
… code when --separate-asm and -s BINARYEN=1 are passed.
juj added a commit to juj/emscripten that referenced this issue Feb 14, 2017
… code when --separate-asm and -s BINARYEN=1 are passed.
@stale
Copy link

stale bot commented Aug 30, 2019

This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Aug 30, 2019
@stale stale bot closed this as completed Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants