Add op_id throughout op API #2734
Conversation
Removes the magic number hack to switch between flatbuffers and the minimal dispatcher. Adds machinery to pass the op_id through the shared_queue.
|
I don't really understand what the purpose of moving the op_id out of the control buffer while leaving the other fields, (e.g. promise_id) in there. |
|
Is this prep for dynamic op dispatching? |
|
@piscisaureus having the op_id put allows us to route to dispatcher - the control buffer contains all the parameters of the various ops - which is not something generally applicable to ops. Like the filename parameter of Deno.stat shouldn’t be visible to Isolate::poll. The name "controlBuf" is probably confusing - it should be called "paramsBuf".
|
|
@afinch7 In part, yes. This allows each op to choose its own serialization mechanism. Something the read/write ops do currently through a very terrible hack: Line 6 in 43d099c In general this patch is a series of updates to the Op API outlined in #2730 |
core/isolate.rs
Outdated
| @@ -283,10 +287,12 @@ impl Isolate { | |||
| // return value. | |||
| // TODO(ry) check that if JSError thrown during respond(), that it will be | |||
| // picked up. | |||
| let _ = isolate.respond(Some(&buf)); | |||
| let op_id = 0; // Sync messages ignore op_id. | |||
| let _ = isolate.respond(Some((op_id, &buf))); | |||
There was a problem hiding this comment.
Maybe panic here if respond() errors?
Ref #2730