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

Unclear how to resolve/reject promises for response based compile API #1081

Closed
jeisinger opened this issue May 26, 2017 · 8 comments
Closed

Comments

@jeisinger
Copy link

Right now, the spec says that the API will run an asynchronous task that eventually resolves the promise. It's a bit unclear to me what an asynchronous task should be, it would be great to explicitly reference an event loop model to explain what this is.

Furthermore, in HTML's event loop model, only regular tasks enter a microtask checkpoint. Without doing this, resolved promise will be left hanging until something else spins the event loop again.

I'd expect that the spec here uses similar terms as e.g. fetch, i.e, when a response promise is passed in and the method doesn't immediately abort, it will finish the compilation asynchronously and when the module is ready create a task on the main event loop and that task resolves the promise.

@jeisinger
Copy link
Author

@domenic can you help with this one?

@domenic
Copy link
Member

domenic commented Jun 14, 2017

Will work on a PR.

domenic added a commit that referenced this issue Jun 14, 2017
Fixes #1081 by being very clear about the interaction with the event loop, i.e. which portions of the tasks happen in parallel and how they get back to the main thread, where objects can actually be created. In general this also makes the specification algorithmic, links it to appropriate definitions, and makes various failure cases (such as being passed a rejected promise, or attempting to read from a locked/disturbed body) clearer.
domenic added a commit that referenced this issue Jul 19, 2017
Fixes #1081 by being very clear about the interaction with the event loop, i.e. which portions of the tasks happen in parallel and how they get back to the main thread, where objects can actually be created. In general this also makes the specification algorithmic, links it to appropriate definitions, and makes various failure cases (such as being passed a rejected promise, or attempting to read from a locked/disturbed body) clearer.
domenic added a commit that referenced this issue Jul 19, 2017
Fixes #1081 by being very clear about the interaction with the event loop, i.e. which portions of the tasks happen in parallel and how they get back to the main thread, where objects can actually be created. In general this also makes the specification algorithmic, links it to appropriate definitions, and makes various failure cases (such as being passed a rejected promise, or attempting to read from a locked/disturbed body) clearer.
@littledan
Copy link

Should JS.md be more explicit in a similar way? @kmiller68 's 3fb9bce does mention "queueing a task" for one of the Promise-returning functions, but it doesn't say on which queue. Should this be the microtask queue?

@domenic
Copy link
Member

domenic commented Oct 22, 2017

Yes, it should be more explicit; no, you cannot enqueue a microtask from a background thread, you need to use a normal task queue.

@littledan
Copy link

littledan commented Oct 23, 2017

Seems like the wording introduced in the patch here makes the start function (part of instantiation) run "in parallel" as well. I assume that was unintentional, right? This would expose multiple pieces of Wasm code running in parallel with each other before threads is there! If this is a mistake, I'll try to fix it, as part of the Bikeshed conversion.

@domenic
Copy link
Member

domenic commented Oct 23, 2017

Ah, yeah, I didn't realize WebAssembly instantiation needed to happen on the main thread. So I guess it's only compilation that makes sense on the background thread. This might significantly simplify the spec, e.g. since now the same steps are performed in parallel by both. Thanks for catching <3

@littledan
Copy link

I'm not sure which parts real implementations do off the main thread, actually. I don't see a reason why steps 1-16 of instantiation couldn't be on another thread; I need to learn more about what implementations do.

@lukewagner
Copy link
Member

Incidentally, I filed design/#1131 about instantiation running in parallel; it could be closed once fixed in the new JS spec.

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