From 046cccfe1768837fcd5b4c1fd7d52fb2d98c0b11 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Mon, 5 Aug 2019 20:12:15 -0400
Subject: [PATCH] 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.
---
 core/isolate.rs      | 65 +++-----------------------------------------
 core/shared_queue.js |  6 +---
 core/shared_queue.rs |  3 ++
 3 files changed, 8 insertions(+), 66 deletions(-)

diff --git a/core/isolate.rs b/core/isolate.rs
index 5df5b4d3c15670..0f693ff92dba1d 100644
--- a/core/isolate.rs
+++ b/core/isolate.rs
@@ -269,34 +269,14 @@ impl Isolate {
     zero_copy_buf: deno_pinned_buf,
   ) {
     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.
-    assert_eq!(isolate.shared.size(), 0);
-
+    debug_assert_eq!(isolate.shared.size(), 0);
     match op {
       Op::Sync(buf) => {
         // For sync messages, we always return the response via Deno.core.send's
@@ -871,43 +851,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.
diff --git a/core/shared_queue.js b/core/shared_queue.js
index d7ab382d6e9317..1b338b052715b4 100644
--- a/core/shared_queue.js
+++ b/core/shared_queue.js
@@ -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 = {
diff --git a/core/shared_queue.rs b/core/shared_queue.rs
index c33a37b90a9c79..616272f8d90e36 100644
--- a/core/shared_queue.rs
+++ b/core/shared_queue.rs
@@ -103,6 +103,7 @@ impl SharedQueue {
     s[INDEX_OFFSETS + index] = end as u32;
   }
 
+  #[cfg(test)]
   fn get_end(&self, index: usize) -> Option<usize> {
     if index < self.num_records() {
       let s = self.as_u32_slice();
@@ -112,6 +113,7 @@ impl SharedQueue {
     }
   }
 
+  #[cfg(test)]
   fn get_offset(&self, index: usize) -> Option<usize> {
     if index < self.num_records() {
       Some(if index == 0 {
@@ -126,6 +128,7 @@ impl SharedQueue {
   }
 
   /// Returns none if empty.
+  #[cfg(test)]
   pub fn shift(&mut self) -> Option<&[u8]> {
     let u32_slice = self.as_u32_slice();
     let i = u32_slice[INDEX_NUM_SHIFTED_OFF] as usize;