Skip to content

Commit cd14e9c

Browse files
committed
Remove dispatch optimization
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.
1 parent ddee2df commit cd14e9c

File tree

2 files changed

+6
-63
lines changed

2 files changed

+6
-63
lines changed

core/isolate.rs

Lines changed: 5 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -268,30 +268,14 @@ impl Isolate {
268268
control_argv0: deno_buf,
269269
zero_copy_buf: deno_pinned_buf,
270270
) {
271+
// The user called Deno.core.send()
272+
271273
let isolate = unsafe { Isolate::from_raw_ptr(user_data) };
272-
let control_shared = isolate.shared.shift();
273274

274-
let op = if control_argv0.len() > 0 {
275-
// The user called Deno.core.send(control)
276-
if let Some(ref f) = isolate.dispatch {
277-
f(control_argv0.as_ref(), PinnedBuf::new(zero_copy_buf))
278-
} else {
279-
panic!("isolate.dispatch not set")
280-
}
281-
} else if let Some(c) = control_shared {
282-
// The user called Deno.sharedQueue.push(control)
283-
if let Some(ref f) = isolate.dispatch {
284-
f(&c, PinnedBuf::new(zero_copy_buf))
285-
} else {
286-
panic!("isolate.dispatch not set")
287-
}
275+
let op = if let Some(ref f) = isolate.dispatch {
276+
f(control_argv0.as_ref(), PinnedBuf::new(zero_copy_buf))
288277
} else {
289-
// The sharedQueue is empty. The shouldn't happen usually, but it's also
290-
// not technically a failure.
291-
#[cfg(test)]
292-
unreachable!();
293-
#[cfg(not(test))]
294-
return;
278+
panic!("isolate.dispatch not set")
295279
};
296280

297281
// At this point the SharedQueue should be empty.
@@ -871,43 +855,6 @@ pub mod tests {
871855
});
872856
}
873857

874-
#[test]
875-
fn test_shared() {
876-
run_in_task(|| {
877-
let (mut isolate, dispatch_count) = setup(Mode::AsyncImmediate);
878-
879-
js_check(isolate.execute(
880-
"setup2.js",
881-
r#"
882-
let nrecv = 0;
883-
Deno.core.setAsyncHandler((buf) => {
884-
assert(buf.byteLength === 1);
885-
assert(buf[0] === 43);
886-
nrecv++;
887-
});
888-
"#,
889-
));
890-
assert_eq!(dispatch_count.load(Ordering::Relaxed), 0);
891-
892-
js_check(isolate.execute(
893-
"send1.js",
894-
r#"
895-
let control = new Uint8Array([42]);
896-
Deno.core.sharedQueue.push(control);
897-
Deno.core.send();
898-
assert(nrecv === 0);
899-
900-
Deno.core.sharedQueue.push(control);
901-
Deno.core.send();
902-
assert(nrecv === 0);
903-
"#,
904-
));
905-
assert_eq!(dispatch_count.load(Ordering::Relaxed), 2);
906-
assert_eq!(Async::Ready(()), isolate.poll().unwrap());
907-
js_check(isolate.execute("send1.js", "assert(nrecv === 2);"));
908-
});
909-
}
910-
911858
#[test]
912859
fn dyn_import_err() {
913860
// Test an erroneous dynamic import where the specified module isn't found.

core/shared_queue.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,7 @@ SharedQueue Binary Layout
165165

166166
function dispatch(control, zeroCopy = null) {
167167
maybeInit();
168-
// First try to push control to shared.
169-
const success = push(control);
170-
// If successful, don't use first argument of core.send.
171-
const arg0 = success ? null : control;
172-
return Deno.core.send(arg0, zeroCopy);
168+
return Deno.core.send(control, zeroCopy);
173169
}
174170

175171
const denoCore = {

0 commit comments

Comments
 (0)