Minimal Worker support #1476
Minimal Worker support #1476
Conversation
This adds the ability to spawn additional Isolates from Rust and send and receive messages from them. This is preliminary work to support running the typescript compiler in a separate isolate and thus support native ES modules (denoland#975).
| Box::new(futures::future::err(err)) | ||
| } | ||
|
|
||
| // https://github.com/denoland/deno/blob/golang/os.go#L100-L154 | ||
| fn op_code_fetch( | ||
| state: &IsolateState, | ||
| state: &Arc<IsolateState>, |
There was a problem hiding this comment.
I wonder if we should refactor so that IsolateState itself has Send, instead of wrapping in an Arc everywhere. After all the Mutex is already internalized...
There was a problem hiding this comment.
Sure. These types of refactors are generally easy to do due to the type system. I'd leave this as is for now because of laziness...
| } | ||
| } | ||
|
|
||
| fn op_worker_get_message( |
There was a problem hiding this comment.
Seeing this does make me feel a bit stronger in favor of a unified messaging substrate instead of having distinct ones.
There was a problem hiding this comment.
We do have a unified messaging substrate; getMessage and postMessage are built on top of the lower-level op dispatch system.
I'd rephrase your suggestion: "postMessage should be the means that we dispatch ops."
I'm not so convinced of that. I think we have very particular concerns in the IPC system that the WebWorkers API doesn't address like synchronous responses, zero-copy buffers.
|
|
||
| /// Rust interface for WebWorkers. | ||
| pub struct Worker { | ||
| isolate: Isolate, |
There was a problem hiding this comment.
Is Isolate itself just a wrapper around a pointer, or a class with lots of data members?
Asking because if the latter is the case, this inflates every entry in the resource table to something potentially huge.
There was a problem hiding this comment.
Yes, it's a class with data members - it's not just a pointer. Your comment is valid.
I agree that the resource table should have Box elements instead of directly containing structs - but I'd rather do that refactor in a different PR.
| pub fn add_worker(wc: WorkerChannels) -> Resource { | ||
| let rid = new_rid(); | ||
| let mut tg = RESOURCE_TABLE.lock().unwrap(); | ||
| let r = tg.insert(rid, Repr::Worker(wc)); |
There was a problem hiding this comment.
Same question as we had once with HTTP -- where does the worker get removed from the resource table?
There was a problem hiding this comment.
They weren't being correctly removed. I've fixed this now. When the thread exits it calls resource.close() which removes it from the table. I've added a test to ensure this.
This adds the ability to spawn additional Isolates from Rust and send
and receive messages from them. This is preliminary work to support
running the typescript compiler in a separate isolate and thus support
native ES modules (#975).
This is a prerequisite commit to #1460