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

WebAssembly.instantiate returning a pair: what attributes do the properties have? #961

Closed
mtrofin opened this issue Jan 20, 2017 · 13 comments

Comments

@mtrofin
Copy link
Contributor

mtrofin commented Jan 20, 2017

If the return of a WebAssembly.instantiate is the pair with the 2 properties - module and instance - what can we say about those properties? Are they read only, for instance?

@mtrofin mtrofin changed the title WebAssembly.instantiate returning a pair: what attributes are the properties? WebAssembly.instantiate returning a pair: what attributes do the properties have? Jan 20, 2017
@rossberg
Copy link
Member

Actually, I think that varying the return type based on the argument is unnecessarily surprising. The functionality is also redundant. This overload is primarily a shortcut for (the majority of) clients who just want the instance. Those that actually need the module can always call W.compile first.

So, I would suggest we just get rid of the pair, thereby simplifying W.instantiate for most users.

@lukewagner
Copy link
Member

I think the high-level logic for any app with a sizable wasm module should be (in pseudocode, pretending everything is sync, ignoring versioning, failure etc):

function createInstance(url, imports) {
  if (idb.has(url))
    return WebAssembly.instantiate(idb.get(url), imports);
  let {module, instance} = WebAssembly.instantiate(fetch(url), imports);
  idb.put(url, module)
  return instance
}

So we definitely need this raw capability and it will be the common case for any beefy app.

But it's fine with me if you want to propose having instantiate always return just the instance and give the {module,instance}-pair-returning method a different name.

@rossberg
Copy link
Member

@lukewagner, fair enough, but this is even slightly less repetitive: :)

function createInstance(url, imports) {
  let has = idb.has(url)
  let module = has ? idb.get(url) : new WebAssembly.compile(fetch(url))
  if (!has) db.put(url, module)
  return new WebAssembly.instantiate(module, imports)
}

(Probably need to make that an async function and sprinkle in the right awaits in both versions.)

Anyway, yeah, if we want this, I'd feel better making it a separate function. I just can't think of a nice name.

@jfbastien
Copy link
Member

Without the Module how would you access Module.{exports,imports,customSection}? We could add Instance.module.

@lukewagner
Copy link
Member

@rossberg-chromium I know, but the resolution of #838 is that WebAssembly.instantiate should be used by default and that pattern would use compile for all cold loads.

@jfbastien The problem with that is it would add a GC edge so that instances would keep their modules alive. The module contains metadata needed for structured cloning and instantiation that you want to be able to GC as soon as instantiation/cloning is complete and the module is unreachable.

@jfbastien
Copy link
Member

@jfbastien The problem with that is it would add a GC edge so that instances would keep their modules alive. The module contains metadata needed for structured cloning and instantiation that you want to be able to GC as soon as instantiation/cloning is complete and the module is unreachable.

What's "that"? Agreed that Instance.module keeps the Module alive.

I'm not proposing anything, I'm asking how one can use the WebAssembly.instantiate API and still access Module.{exports,imports,customSection}. My impression is that this is important. Do you think otherwise?

Given your assessment, which solution would you propose?

@lukewagner
Copy link
Member

lukewagner commented Jan 20, 2017

@jfbastien "That" would be adding a .module property to instances. At the moment (in JS.md and in FF), there isn't any problem: when you call instantiate you get back both the module and instance as a pair.

@jfbastien
Copy link
Member

@lukewagner sure. That still doesn't resolve the question I ask.

@lukewagner
Copy link
Member

lukewagner commented Jan 20, 2017

let {module, instance} = WebAssembly.instantiate(bytes, imports);
var imports = WebAssembly.Module.imports(module);

@jfbastien
Copy link
Member

@lukewagner that won't work if WebAssembly.instantiate doesn't return the pair, as is being suggested. I'm not sure there's a point in having two version, one returning the pair and one not, given that ignoring one of the two returned values is trivial.

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 20, 2017

I thought that the motivation, alternatives, and design for this were discussed in #838.

Assuming we stay the course with the current API - any stance on what the properties should be attributed as?

@lukewagner
Copy link
Member

lukewagner commented Jan 20, 2017

@jfbastien Ah, I was already assuming we're keeping the returned pair and only considering the more superficial change of renaming. Personally, I'd rather not, but I'm not all that familiar with what the precedent is for overloads like this.

@mtrofin Sorry your root question was lost :) The current wording "On success, the Promise is fulfilled with a plain JavaScript object pair {module, instance}" was intended to define the returned objects' properties order and descriptors as if the analogous object literal was evaluated. Thus the order is module then instance (although ISTR that property enumeration order isn't fully defined?) and the properties are data properties that are configureable, enumerable and writable.

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 20, 2017

Thanks!

@mtrofin mtrofin closed this as completed Jan 20, 2017
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

No branches or pull requests

4 participants