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

wasm import: remove double compilation #3348

Closed
kevinkassimo opened this issue Nov 14, 2019 · 3 comments
Closed

wasm import: remove double compilation #3348

kevinkassimo opened this issue Nov 14, 2019 · 3 comments
Labels
bug Something isn't working correctly

Comments

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Nov 14, 2019

Follow-up for #3328

wasm import currently does double compilation: the first compilation happens here:

const compiled = await WebAssembly.compile(buffer);

and the second time happens here:

const compiled = await WebAssembly.compile(buffer);

The first compilation is to enable us to get imports/exports info such that we are able to build a dynamic script with static imports and named exports. The second one is for the later actual instantiation.

Ideally we should be able to just compile once, and in the module evaluation script we can somehow get a handle to the old compiled object. However currently these 2 compilations happens in 2 different isolates (first in some compiler isolate, second in the main isolate), thus making storing and transferring the compiled object difficult.

I'm not so sure if SyntheticModule APIs could be useful, since handling imports in its evaluation steps would be painful while doing exports is easy. Also we still need to create some js/.d.ts code for the module to placate TS compiler for named exports (especially under de-structuring)

@kevinkassimo
Copy link
Contributor Author

For clarification, this is a generated JS wrapper script for wasm imports (with cli/tests/051_wasm_import/simple.wasm):

const importObject = Object.create(null);
/* BEGIN static import inject */
import * as m0 from "./wasm-dep.js";
importObject["./wasm-dep.js"] = m0;
import * as m1 from "http://127.0.0.1:4545/cli/tests/051_wasm_import/remote.ts";
importObject["http://127.0.0.1:4545/cli/tests/051_wasm_import/remote.ts"] = m1;
/* END static import inject */

function base64ToUint8Array(data) {
  const binString = window.atob(data);
  const size = binString.length;
  const bytes = new Uint8Array(size);
  for (let i = 0; i < size; i++) {
    bytes[i] = binString.charCodeAt(i);
  }
  return bytes;
}

const buffer = base64ToUint8Array("AGFzbQEAAAABEwRgAAF/YAAAYAJ/fwF/YAF/AX8CdgMNLi93YXNtLWRlcC5qcwRqc0ZuAAANLi93YXNtLWRlcC5qcwhqc0luaXRGbgABOWh0dHA6Ly8xMjcuMC4wLjE6NDU0NS9jbGkvdGVzdHMvMDUxX3dhc21faW1wb3J0L3JlbW90ZS50cwpqc1JlbW90ZUZuAAADBQQBAgMDByEDA2FkZAAEC2FkZEltcG9ydGVkAAUJYWRkUmVtb3RlAAYIAQMKHgQEABABCwcAIAAgAWoLBwAgABAAagsHACAAEAJqCw==");
const compiled = await WebAssembly.compile(buffer);

const instance = new WebAssembly.Instance(compiled, importObject);

/* BEGIN named exports inject */
export const add = instance.exports.add;
export const addImported = instance.exports.addImported;
export const addRemote = instance.exports.addRemote;
/* END named exports inject */

@ry ry added the bug Something isn't working correctly label Nov 14, 2019
@SyrupThinker
Copy link
Contributor

This can probably be closed with the removal of wasm imports in 2b66b8a.

@bartlomieju
Copy link
Member

Closing in favor of #5609

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants