Skip to content

Commit

Permalink
Remove dispatch optimization
Browse files Browse the repository at this point in the history
Deno.core.dispatch() used to push the "control" buf onto the shared
array buffer before calling into V8, with the idea that it was one less
argument to parse. Turns out there is no more overhead passing the
control ArrayBuffer directly over. Furthermore this optimization was
making the refactors outlined in denoland#2730 more complex. Therefore it is
being removed.
  • Loading branch information
ry committed Aug 5, 2019
1 parent ddee2df commit cd14e9c
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 63 deletions.
63 changes: 5 additions & 58 deletions core/isolate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,30 +268,14 @@ impl Isolate {
control_argv0: deno_buf,
zero_copy_buf: deno_pinned_buf,
) {
// The user called Deno.core.send()

let isolate = unsafe { Isolate::from_raw_ptr(user_data) };
let control_shared = isolate.shared.shift();

let op = if control_argv0.len() > 0 {
// The user called Deno.core.send(control)
if let Some(ref f) = isolate.dispatch {
f(control_argv0.as_ref(), PinnedBuf::new(zero_copy_buf))
} else {
panic!("isolate.dispatch not set")
}
} else if let Some(c) = control_shared {
// The user called Deno.sharedQueue.push(control)
if let Some(ref f) = isolate.dispatch {
f(&c, PinnedBuf::new(zero_copy_buf))
} else {
panic!("isolate.dispatch not set")
}
let op = if let Some(ref f) = isolate.dispatch {
f(control_argv0.as_ref(), PinnedBuf::new(zero_copy_buf))
} else {
// The sharedQueue is empty. The shouldn't happen usually, but it's also
// not technically a failure.
#[cfg(test)]
unreachable!();
#[cfg(not(test))]
return;
panic!("isolate.dispatch not set")
};

// At this point the SharedQueue should be empty.
Expand Down Expand Up @@ -871,43 +855,6 @@ pub mod tests {
});
}

#[test]
fn test_shared() {
run_in_task(|| {
let (mut isolate, dispatch_count) = setup(Mode::AsyncImmediate);

js_check(isolate.execute(
"setup2.js",
r#"
let nrecv = 0;
Deno.core.setAsyncHandler((buf) => {
assert(buf.byteLength === 1);
assert(buf[0] === 43);
nrecv++;
});
"#,
));
assert_eq!(dispatch_count.load(Ordering::Relaxed), 0);

js_check(isolate.execute(
"send1.js",
r#"
let control = new Uint8Array([42]);
Deno.core.sharedQueue.push(control);
Deno.core.send();
assert(nrecv === 0);
Deno.core.sharedQueue.push(control);
Deno.core.send();
assert(nrecv === 0);
"#,
));
assert_eq!(dispatch_count.load(Ordering::Relaxed), 2);
assert_eq!(Async::Ready(()), isolate.poll().unwrap());
js_check(isolate.execute("send1.js", "assert(nrecv === 2);"));
});
}

#[test]
fn dyn_import_err() {
// Test an erroneous dynamic import where the specified module isn't found.
Expand Down
6 changes: 1 addition & 5 deletions core/shared_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,7 @@ SharedQueue Binary Layout

function dispatch(control, zeroCopy = null) {
maybeInit();
// First try to push control to shared.
const success = push(control);
// If successful, don't use first argument of core.send.
const arg0 = success ? null : control;
return Deno.core.send(arg0, zeroCopy);
return Deno.core.send(control, zeroCopy);
}

const denoCore = {
Expand Down

0 comments on commit cd14e9c

Please sign in to comment.