Skip to content

Commit

Permalink
remove legacy Isolate.set_dispatch API (#3041)
Browse files Browse the repository at this point in the history 
* migrate deno_typescript crate to Isolate.register_op API
* remove dual-dispatch mechanism
* update Isolate tests to new dispatch mechanism
  • Loading branch information
@bartlomieju @ry
bartlomieju authored and ry committed Oct 2, 2019
1 parent 99eec73 commit a569be8
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 148 deletions.
157 changes: 65 additions & 92 deletions core/isolate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ use std::fmt;
use std::ptr::null;
use std::sync::{Arc, Mutex, Once};

/// Args: op_id, control_buf, zero_copy_buf
type CoreDispatchFn = dyn Fn(OpId, &[u8], Option<PinnedBuf>) -> CoreOp;

/// Stores a script used to initalize a Isolate
pub struct Script<'a> {
pub source: &'a str,
Expand Down Expand Up @@ -147,12 +144,11 @@ type JSErrorCreateFn = dyn Fn(V8Exception) -> ErrBox;
/// pending ops have completed.
///
/// Ops are created in JavaScript by calling Deno.core.dispatch(), and in Rust
/// by implementing deno::Dispatch::dispatch. An async Op corresponds exactly to
/// a Promise in JavaScript.
/// by implementing dispatcher function that takes control buffer and optional zero copy buffer
/// as arguments. An async Op corresponds exactly to a Promise in JavaScript.
pub struct Isolate {
libdeno_isolate: *const libdeno::isolate,
shared_libdeno_isolate: Arc<Mutex<Option<*const libdeno::isolate>>>,
dispatch: Option<Arc<CoreDispatchFn>>,
dyn_import: Option<Arc<DynImportFn>>,
js_error_create: Arc<JSErrorCreateFn>,
needs_init: bool,
Expand Down Expand Up @@ -218,7 +214,6 @@ impl Isolate {
Self {
libdeno_isolate,
shared_libdeno_isolate: Arc::new(Mutex::new(Some(libdeno_isolate))),
dispatch: None,
dyn_import: None,
js_error_create: Arc::new(CoreJSError::from_v8_exception),
shared,
Expand All @@ -235,29 +230,11 @@ impl Isolate {
/// Called whenever Deno.core.dispatch() is called in JavaScript. zero_copy_buf
/// corresponds to the second argument of Deno.core.dispatch().
///
/// If this method is used then ops registered using `op_register` function are
/// ignored and all dispatching must be handled manually in provided callback.
// TODO: we want to deprecate and remove this API and move to `register_op` API
pub fn set_dispatch<F>(&mut self, f: F)
where
F: Fn(OpId, &[u8], Option<PinnedBuf>) -> CoreOp + Send + Sync + 'static,
{
self.dispatch = Some(Arc::new(f));
}

/// New dispatch mechanism. Requires runtime to explicitly ask for op ids
/// before using any of the ops.
///
/// Ops added using this method are only usable if `dispatch` is not set
/// (using `set_dispatch` method).
/// Requires runtime to explicitly ask for op ids before using any of the ops.
pub fn register_op<F>(&mut self, name: &str, op: F) -> OpId
where
F: Fn(&[u8], Option<PinnedBuf>) -> CoreOp + Send + Sync + 'static,
{
assert!(
self.dispatch.is_none(),
"set_dispatch should not be used in conjunction with register_op"
);
self.op_registry.register(name, op)
}

Expand Down Expand Up @@ -332,19 +309,11 @@ impl Isolate {
) {
let isolate = unsafe { Isolate::from_raw_ptr(user_data) };

let op = if let Some(ref f) = isolate.dispatch {
assert!(
op_id != 0,
"op_id 0 is a special value that shouldn't be used with dispatch"
);
f(op_id, control_buf.as_ref(), PinnedBuf::new(zero_copy_buf))
} else {
isolate.op_registry.call(
op_id,
control_buf.as_ref(),
PinnedBuf::new(zero_copy_buf),
)
};
let op = isolate.op_registry.call(
op_id,
control_buf.as_ref(),
PinnedBuf::new(zero_copy_buf),
);

debug_assert_eq!(isolate.shared.size(), 0);
match op {
Expand Down Expand Up @@ -792,46 +761,50 @@ pub mod tests {
let dispatch_count_ = dispatch_count.clone();

let mut isolate = Isolate::new(StartupData::None, false);
isolate.set_dispatch(move |op_id, control, _| -> CoreOp {
println!("op_id {}", op_id);
dispatch_count_.fetch_add(1, Ordering::Relaxed);
match mode {
Mode::AsyncImmediate => {
assert_eq!(control.len(), 1);
assert_eq!(control[0], 42);
let buf = vec![43u8, 0, 0, 0].into_boxed_slice();
Op::Async(Box::new(futures::future::ok(buf)))
}
Mode::OverflowReqSync => {
assert_eq!(control.len(), 100 * 1024 * 1024);
let buf = vec![43u8, 0, 0, 0].into_boxed_slice();
Op::Sync(buf)
}
Mode::OverflowResSync => {
assert_eq!(control.len(), 1);
assert_eq!(control[0], 42);
let mut vec = Vec::<u8>::new();
vec.resize(100 * 1024 * 1024, 0);
vec[0] = 99;
let buf = vec.into_boxed_slice();
Op::Sync(buf)
}
Mode::OverflowReqAsync => {
assert_eq!(control.len(), 100 * 1024 * 1024);
let buf = vec![43u8, 0, 0, 0].into_boxed_slice();
Op::Async(Box::new(futures::future::ok(buf)))
}
Mode::OverflowResAsync => {
assert_eq!(control.len(), 1);
assert_eq!(control[0], 42);
let mut vec = Vec::<u8>::new();
vec.resize(100 * 1024 * 1024, 0);
vec[0] = 4;
let buf = vec.into_boxed_slice();
Op::Async(Box::new(futures::future::ok(buf)))

let dispatcher =
move |control: &[u8], _zero_copy: Option<PinnedBuf>| -> CoreOp {
dispatch_count_.fetch_add(1, Ordering::Relaxed);
match mode {
Mode::AsyncImmediate => {
assert_eq!(control.len(), 1);
assert_eq!(control[0], 42);
let buf = vec![43u8, 0, 0, 0].into_boxed_slice();
Op::Async(Box::new(futures::future::ok(buf)))
}
Mode::OverflowReqSync => {
assert_eq!(control.len(), 100 * 1024 * 1024);
let buf = vec![43u8, 0, 0, 0].into_boxed_slice();
Op::Sync(buf)
}
Mode::OverflowResSync => {
assert_eq!(control.len(), 1);
assert_eq!(control[0], 42);
let mut vec = Vec::<u8>::new();
vec.resize(100 * 1024 * 1024, 0);
vec[0] = 99;
let buf = vec.into_boxed_slice();
Op::Sync(buf)
}
Mode::OverflowReqAsync => {
assert_eq!(control.len(), 100 * 1024 * 1024);
let buf = vec![43u8, 0, 0, 0].into_boxed_slice();
Op::Async(Box::new(futures::future::ok(buf)))
}
Mode::OverflowResAsync => {
assert_eq!(control.len(), 1);
assert_eq!(control[0], 42);
let mut vec = Vec::<u8>::new();
vec.resize(100 * 1024 * 1024, 0);
vec[0] = 4;
let buf = vec.into_boxed_slice();
Op::Async(Box::new(futures::future::ok(buf)))
}
}
}
});
};

isolate.register_op("test", dispatcher);

js_check(isolate.execute(
"setup.js",
r#"
Expand All @@ -853,9 +826,9 @@ pub mod tests {
"filename.js",
r#"
let control = new Uint8Array([42]);
Deno.core.send(42, control);
Deno.core.send(1, control);
async function main() {
Deno.core.send(42, control);
Deno.core.send(1, control);
}
main();
"#,
Expand All @@ -874,7 +847,7 @@ pub mod tests {
import { b } from 'b.js'
if (b() != 'b') throw Error();
let control = new Uint8Array([42]);
Deno.core.send(42, control);
Deno.core.send(1, control);
"#,
)
.unwrap();
Expand Down Expand Up @@ -931,7 +904,7 @@ pub mod tests {
r#"
assert(nrecv == 0);
let control = new Uint8Array([42]);
Deno.core.send(42, control);
Deno.core.send(1, control);
assert(nrecv == 0);
"#,
));
Expand All @@ -942,7 +915,7 @@ pub mod tests {
"check2.js",
r#"
assert(nrecv == 1);
Deno.core.send(42, control);
Deno.core.send(1, control);
assert(nrecv == 1);
"#,
));
Expand Down Expand Up @@ -1223,7 +1196,7 @@ pub mod tests {
Deno.core.setAsyncHandler((opId, buf) => { asyncRecv++ });
// Large message that will overflow the shared space.
let control = new Uint8Array(100 * 1024 * 1024);
let response = Deno.core.dispatch(99, control);
let response = Deno.core.dispatch(1, control);
assert(response instanceof Uint8Array);
assert(response.length == 4);
assert(response[0] == 43);
Expand All @@ -1245,7 +1218,7 @@ pub mod tests {
Deno.core.setAsyncHandler((opId, buf) => { asyncRecv++ });
// Large message that will overflow the shared space.
let control = new Uint8Array([42]);
let response = Deno.core.dispatch(99, control);
let response = Deno.core.dispatch(1, control);
assert(response instanceof Uint8Array);
assert(response.length == 100 * 1024 * 1024);
assert(response[0] == 99);
Expand All @@ -1264,14 +1237,14 @@ pub mod tests {
r#"
let asyncRecv = 0;
Deno.core.setAsyncHandler((opId, buf) => {
assert(opId == 99);
assert(opId == 1);
assert(buf.byteLength === 4);
assert(buf[0] === 43);
asyncRecv++;
});
// Large message that will overflow the shared space.
let control = new Uint8Array(100 * 1024 * 1024);
let response = Deno.core.dispatch(99, control);
let response = Deno.core.dispatch(1, control);
// Async messages always have null response.
assert(response == null);
assert(asyncRecv == 0);
Expand All @@ -1294,14 +1267,14 @@ pub mod tests {
r#"
let asyncRecv = 0;
Deno.core.setAsyncHandler((opId, buf) => {
assert(opId == 99);
assert(opId == 1);
assert(buf.byteLength === 100 * 1024 * 1024);
assert(buf[0] === 4);
asyncRecv++;
});
// Large message that will overflow the shared space.
let control = new Uint8Array([42]);
let response = Deno.core.dispatch(99, control);
let response = Deno.core.dispatch(1, control);
assert(response == null);
assert(asyncRecv == 0);
"#,
Expand All @@ -1323,19 +1296,19 @@ pub mod tests {
r#"
let asyncRecv = 0;
Deno.core.setAsyncHandler((opId, buf) => {
assert(opId === 99);
assert(opId === 1);
assert(buf.byteLength === 100 * 1024 * 1024);
assert(buf[0] === 4);
asyncRecv++;
});
// Large message that will overflow the shared space.
let control = new Uint8Array([42]);
let response = Deno.core.dispatch(99, control);
let response = Deno.core.dispatch(1, control);
assert(response == null);
assert(asyncRecv == 0);
// Dispatch another message to verify that pending ops
// are done even if shared space overflows
Deno.core.dispatch(99, control);
Deno.core.dispatch(1, control);
"#,
));
assert_eq!(dispatch_count.load(Ordering::Relaxed), 2);
Expand Down
16 changes: 8 additions & 8 deletions core/shared_queue_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ function fullRecords(q) {
q.reset();
const oneByte = new Uint8Array([42]);
for (let i = 0; i < q.MAX_RECORDS; i++) {
assert(q.push(99, oneByte));
assert(q.push(1, oneByte));
}
assert(!q.push(99, oneByte));
assert(!q.push(1, oneByte));
const [opId, r] = q.shift();
assert(opId == 99);
assert(opId == 1);
assert(r.byteLength == 1);
assert(r[0] == 42);
// Even if we shift one off, we still cannot push a new record.
assert(!q.push(99, oneByte));
assert(!q.push(1, oneByte));
}

function main() {
Expand All @@ -30,14 +30,14 @@ function main() {

let r = new Uint8Array([1, 2, 3, 4, 5]);
const len = r.byteLength + h;
assert(q.push(99, r));
assert(q.push(1, r));
assert(q.head() == len);

r = new Uint8Array([6, 7]);
assert(q.push(99, r));
assert(q.push(1, r));

r = new Uint8Array([8, 9, 10, 11]);
assert(q.push(99, r));
assert(q.push(1, r));
assert(q.numRecords() == 3);
assert(q.size() == 3);

Expand All @@ -60,7 +60,7 @@ function main() {
assert(q.size() == 1);

[opId, r] = q.shift();
assert(opId == 99);
assert(opId == 1);
assert(r.byteLength == 4);
assert(r[0] == 8);
assert(r[1] == 9);
Expand Down
21 changes: 11 additions & 10 deletions deno_typescript/compiler_main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const ASSETS = "$asset$";
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
function main(configText, rootNames) {
ops = Deno.core.ops();
println(`>>> ts version ${ts.version}`);
println(`>>> rootNames ${rootNames}`);

Expand Down Expand Up @@ -97,17 +98,11 @@ function encode(str) {
}

//
/** **Warning!** The op_id values below are shared between this code and the
* Rust side. Update with care!
/** **Warning!** Op ids must be acquired from Rust using `Deno.core.ops()`
* before dispatching any action.
* @type {Record<string, number>}
*/
const ops = {
readFile: 49,
exit: 50,
writeFile: 51,
resolveModuleNames: 52,
setEmitResult: 53
};
let ops;

/**
* @type {Map<string, string>}
Expand Down Expand Up @@ -315,9 +310,15 @@ function configure(configurationText) {
* @param {Record<string,any>} obj
*/
function dispatch(opName, obj) {
const opId = ops[opName];

if (!opId) {
throw new Error(`Unknown op: ${opName}`);
}

const s = JSON.stringify(obj);
const msg = encode(s);
const resUi8 = Deno.core.dispatch(ops[opName], msg);
const resUi8 = Deno.core.dispatch(opId, msg);
const resStr = decodeAscii(resUi8);
const res = JSON.parse(resStr);
if (!res["ok"]) {
Expand Down
Loading

0 comments on commit a569be8

Please sign in to comment.