Skip to content

Commit 046cccf

Browse files
authored
Remove dispatch optimization (#2732)
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 #2730 more complex. Therefore it is being removed.
1 parent a517513 commit 046cccf

File tree

3 files changed

+8
-66
lines changed

3 files changed

+8
-66
lines changed

core/isolate.rs

Lines changed: 4 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -269,34 +269,14 @@ impl Isolate {
269269
zero_copy_buf: deno_pinned_buf,
270270
) {
271271
let isolate = unsafe { Isolate::from_raw_ptr(user_data) };
272-
let control_shared = isolate.shared.shift();
273272

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-
}
273+
let op = if let Some(ref f) = isolate.dispatch {
274+
f(control_argv0.as_ref(), PinnedBuf::new(zero_copy_buf))
288275
} 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;
276+
panic!("isolate.dispatch not set")
295277
};
296278

297-
// At this point the SharedQueue should be empty.
298-
assert_eq!(isolate.shared.size(), 0);
299-
279+
debug_assert_eq!(isolate.shared.size(), 0);
300280
match op {
301281
Op::Sync(buf) => {
302282
// For sync messages, we always return the response via Deno.core.send's
@@ -871,43 +851,6 @@ pub mod tests {
871851
});
872852
}
873853

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-
911854
#[test]
912855
fn dyn_import_err() {
913856
// 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 = {

core/shared_queue.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ impl SharedQueue {
103103
s[INDEX_OFFSETS + index] = end as u32;
104104
}
105105

106+
#[cfg(test)]
106107
fn get_end(&self, index: usize) -> Option<usize> {
107108
if index < self.num_records() {
108109
let s = self.as_u32_slice();
@@ -112,6 +113,7 @@ impl SharedQueue {
112113
}
113114
}
114115

116+
#[cfg(test)]
115117
fn get_offset(&self, index: usize) -> Option<usize> {
116118
if index < self.num_records() {
117119
Some(if index == 0 {
@@ -126,6 +128,7 @@ impl SharedQueue {
126128
}
127129

128130
/// Returns none if empty.
131+
#[cfg(test)]
129132
pub fn shift(&mut self) -> Option<&[u8]> {
130133
let u32_slice = self.as_u32_slice();
131134
let i = u32_slice[INDEX_NUM_SHIFTED_OFF] as usize;

0 commit comments

Comments
 (0)