Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

wasi: improve JavaScript API #4

Merged
merged 2 commits into from
Sep 11, 2019
Merged

wasi: improve JavaScript API #4

merged 2 commits into from
Sep 11, 2019

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Sep 9, 2019

  • Add WASI.start(). This calls _start() or __wasi_unstable_reactor_start(), if present.
  • Export a valid wasi_unstable import. This can be included as part of an imports object passed to WebAssembly.instantiate(), etc.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

- Add WASI.start().
- Export a valid wasi_unstable import.
this.wasiImport = wrap;
}

static start(instance) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if instance.memory !== options.memory passed to new WASI? Should this be considered an API hazard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since start() is a static method, I don't think that should matter, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right I see, but then should there be some memory binding in this method instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I'm assuming that WASI.start() is not the same thing as the API being discussed in #3. I'm thinking of WASI.start() as more of a building block API, and #3 as a higher level API.

With that assumption, consider the following example:

'use strict';
const fs = require('fs');
const { WASI } = require('wasi');
const pathname = '/path/to/something.wasm';
const buffer = fs.readFileSync(pathname);
const memory = new WebAssembly.Memory({ initial: 1 });
const wasi = new WASI({ args: [], env: process.env, memory });

(async () => {
  const compiled = await WebAssembly.compile(buffer);
  const importObject = { wasi_unstable: wasi.wasiImport };
  const instance = await WebAssembly.instantiate(compiled, importObject);

  WASI.start(instance);
})();

In this case, WASI.start() just attempts to call the _start() or __wasi_unstable_reactor_start() export if one exists. By this point, instance should have everything bound properly, or WebAssembly.instantiate() will fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. For commands that export their memories, a memory injection API would still likely be needed though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I plan to have a WASI.prototype.setMemory() (or something along those lines) eventually.

Copy link
Member

Choose a reason for hiding this comment

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

If setMemory can take a SharedArrayBuffer that could be awesome (although I am not sure if that is allowed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed up 2b0131c to this PR. It allows the memory passed to the WASI constructor to be a SharedArrayBuffer (or any other ArrayBuffer). Once setMemory() is added, it will also support SABs.

Copy link
Member

Choose a reason for hiding this comment

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

This is really cool and useful thanks :]

This commit adds support for SharedArrayBuffers as the WASI
memory.
@danbev danbev mentioned this pull request Sep 11, 2019
2 tasks
@cjihrig cjihrig merged commit 2b0131c into nodejs:wasi Sep 11, 2019
@cjihrig cjihrig deleted the wasi-2 branch September 11, 2019 13:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants