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

Common JS shell infrastructure #52

Open
lars-t-hansen opened this issue Jul 18, 2017 · 10 comments
Open

Common JS shell infrastructure #52

lars-t-hansen opened this issue Jul 18, 2017 · 10 comments
Assignees

Comments

@lars-t-hansen
Copy link

cc @jfbastien @saambarati @kmiller68 @binji @MikeHolman

JF brought up the issue that we'll want a shared system in the JS shells to be able to test wasm threaded code. (And JS code for that matter - everyone will love us.)

To kick us off, I propose the following strawman:

The main thread can create new workers:

var w = new Worker(text)  // where text is defined below
w.postMessage(obj)           // what you think
w.onmessage = function (ev) { ... } // where ev.data is something sent by postMessage
dispatch()                           // enter the event loop

The worker can then:

onmessage = function (ev) { ... } where ev.data is something sent by postMessage
postMessage(obj)       // what you think

Note,

  • There is an implicit event loop in the worker.
  • There is an explicit event loop in the main thread by means of dispatch(); when it returns the main thread continues and may exit. (Debatable how good this explicit event loop is, not sure what d8 does here. Discuss.)
  • The worker cannot create new workers (keeps things simple).
  • The object to be posted is anything structured-cloneable
  • I haven't bothered to worry about transfering objects here

The "text" for a worker is a string that starts with 'text/plain;', this is for web compat so that tests can just carry over (ideally).

@binji
Copy link
Member

binji commented Jul 18, 2017

This is pretty close to what is currently implemented in d8. The primary difference is that there is currently no event loop on the main thread. Instead, messages are queued:

// Main thread.
var w = new Worker(text);
w.postMessage(obj);
w.getMessage();  // Get object from worker, or undefined if none available.
w.terminate();

// Worker.
postMessage(obj);
onmessage = function(m) {
  ...
};
  • There is an implicit event loop in the worker.
  • There is no event loop on the main thread.
  • The worker can't create new workers
  • Objects posted must be structured-cloneable or transferable
  • Text is just raw text.

@lars-t-hansen
Copy link
Author

@binji, so you just end up busy-waiting on the main thread? Or do you also have a sleep() like the SpiderMonkey shell has? Busy-waiting is a little nasty but I suppose we can live with it if we must.

It looks like for d8 one could perhaps create a dispatch function that just gets messages from the workers (round-robin with some kind of relaxation?) and dispatches them to installed onmessage handlers, if we wanted to find common ground and be closer to the web platform.

@binji
Copy link
Member

binji commented Jul 19, 2017

Yeah, the tests just spin wait. But @mtrofin pointed out that we've recently added support for using an event loop on the main thread, so perhaps we can just rework to use that.

@mtrofin
Copy link
Contributor

mtrofin commented Jul 19, 2017

We have a testRunner.waitUntilDone() and testRunner.notifyWait(), akin what's used in LayoutTests. The former starts an event loop on the main thread. The latter ends it when all pending work is done.

We don't busy-wait, instead, we wait on a semaphore to get more work.

@saambarati
Copy link

@lars-t-hansen This proposal looks good to me. I like it being similar to web spec.

What dictates when the event loop can return back inside dispatch()?

@lars-t-hansen
Copy link
Author

@saambarati, I think we need something like the v8 infrastructure has, eg, a returnFromDispatch() function that can be invoked by one of the registered event handlers on the main thread when it discovers that the execution can end.

I think that with an event loop on the main thread we won't need the w.getMessage() that the v8 shell has; that seems to be an artifact of not having an event loop previously.

@lars-t-hansen
Copy link
Author

I've been stalled on this issue, no work since July. Hope to get to it soon.

@lars-t-hansen
Copy link
Author

There's a concrete API description (based closely on the discussion above) and a sample implementation for the SpiderMonkey shell here: https://github.com/lars-t-hansen/Worker. See the file API.md in that repo. Note the "Notes" and "Questions" sections in that doc.

@saambarati, @binji, @kmiller68, @mtrofin, @MikeHolman - is this something you can support in your shells? Obviously we can bicker about names, notably enterEventLoop() and exitEventLoop() are new.

@binji, do we really need a w.getMessage() if we have event handling on both sides?

As far as the discussion is concerned, probably best to keep it in this ticket, so that we have it all in one place.

@jfbastien
Copy link
Member

We'll track our implementation here: https://bugs.webkit.org/show_bug.cgi?id=179319

@binji
Copy link
Member

binji commented Nov 7, 2017

@lars-t-hansen No, getMessage isn't necessary. I added it because we didn't have event loop support in d8 at the time, but it would be better to have the API more closely match the browser, IMO.

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

5 participants