From 136b5e3da2c689b54b34c46fa41973e0ccca66ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Mon, 2 Dec 2019 21:21:33 +0100
Subject: [PATCH] disable eager polling for ops (#3434)

---
 cli/ops/net.rs  | 26 +-------------------------
 cli/ops/tls.rs  | 26 +-------------------------
 core/isolate.rs | 26 --------------------------
 3 files changed, 2 insertions(+), 76 deletions(-)

diff --git a/cli/ops/net.rs b/cli/ops/net.rs
index 929b87dde1..6d843c0ba2 100644
--- a/cli/ops/net.rs
+++ b/cli/ops/net.rs
@@ -33,7 +33,6 @@ pub fn init(i: &mut Isolate, s: &ThreadSafeState) {
 
 #[derive(Debug, PartialEq)]
 enum AcceptState {
-  Eager,
   Pending,
   Done,
 }
@@ -41,7 +40,7 @@ enum AcceptState {
 /// Simply accepts a connection.
 pub fn accept(state: &ThreadSafeState, rid: ResourceId) -> Accept {
   Accept {
-    accept_state: AcceptState::Eager,
+    accept_state: AcceptState::Pending,
     rid,
     state: state.clone(),
   }
@@ -78,29 +77,6 @@ impl Future for Accept {
       futures::compat::Compat01As03::new(&mut listener_resource.listener)
         .map_err(ErrBox::from);
 
-    if inner.accept_state == AcceptState::Eager {
-      // Similar to try_ready!, but also track/untrack accept task
-      // in TcpListener resource.
-      // In this way, when the listener is closed, the task can be
-      // notified to error out (instead of stuck forever).
-      match listener.poll_next_unpin(cx) {
-        Poll::Ready(Some(Ok(stream))) => {
-          inner.accept_state = AcceptState::Done;
-          let addr = stream.peer_addr().unwrap();
-          return Poll::Ready(Ok((stream, addr)));
-        }
-        Poll::Pending => {
-          inner.accept_state = AcceptState::Pending;
-          return Poll::Pending;
-        }
-        Poll::Ready(Some(Err(e))) => {
-          inner.accept_state = AcceptState::Done;
-          return Poll::Ready(Err(e));
-        }
-        _ => unreachable!(),
-      }
-    }
-
     match listener.poll_next_unpin(cx) {
       Poll::Ready(Some(Ok(stream))) => {
         listener_resource.untrack_task();
diff --git a/cli/ops/tls.rs b/cli/ops/tls.rs
index 484b7057cc..33900d4e0f 100644
--- a/cli/ops/tls.rs
+++ b/cli/ops/tls.rs
@@ -303,7 +303,6 @@ fn op_listen_tls(
 
 #[derive(Debug, PartialEq)]
 enum AcceptTlsState {
-  Eager,
   Pending,
   Done,
 }
@@ -311,7 +310,7 @@ enum AcceptTlsState {
 /// Simply accepts a TLS connection.
 pub fn accept_tls(state: &ThreadSafeState, rid: ResourceId) -> AcceptTls {
   AcceptTls {
-    accept_state: AcceptTlsState::Eager,
+    accept_state: AcceptTlsState::Pending,
     rid,
     state: state.clone(),
   }
@@ -348,29 +347,6 @@ impl Future for AcceptTls {
       futures::compat::Compat01As03::new(&mut listener_resource.listener)
         .map_err(ErrBox::from);
 
-    if inner.accept_state == AcceptTlsState::Eager {
-      // Similar to try_ready!, but also track/untrack accept task
-      // in TcpListener resource.
-      // In this way, when the listener is closed, the task can be
-      // notified to error out (instead of stuck forever).
-      match listener.poll_next_unpin(cx) {
-        Poll::Ready(Some(Ok(stream))) => {
-          inner.accept_state = AcceptTlsState::Done;
-          let addr = stream.peer_addr().unwrap();
-          return Poll::Ready(Ok((stream, addr)));
-        }
-        Poll::Pending => {
-          inner.accept_state = AcceptTlsState::Pending;
-          return Poll::Pending;
-        }
-        Poll::Ready(Some(Err(e))) => {
-          inner.accept_state = AcceptTlsState::Done;
-          return Poll::Ready(Err(e));
-        }
-        _ => unreachable!(),
-      }
-    }
-
     match listener.poll_next_unpin(cx) {
       Poll::Ready(Some(Ok(stream))) => {
         listener_resource.untrack_task();
diff --git a/core/isolate.rs b/core/isolate.rs
index 41c8b02fd9..06abfc0ef0 100644
--- a/core/isolate.rs
+++ b/core/isolate.rs
@@ -179,7 +179,6 @@ pub struct Isolate {
   have_unpolled_ops: bool,
   startup_script: Option<OwnedScript>,
   pub op_registry: Arc<OpRegistry>,
-  eager_poll_count: u32,
   waker: AtomicWaker,
 }
 
@@ -246,7 +245,6 @@ impl Isolate {
       pending_dyn_imports: FuturesUnordered::new(),
       startup_script,
       op_registry: Arc::new(OpRegistry::new()),
-      eager_poll_count: 0,
       waker: AtomicWaker::new(),
     }
   }
@@ -349,29 +347,6 @@ impl Isolate {
       }
     };
 
-    // To avoid latency problems we eagerly poll 50 futures and then
-    // allow to poll ops from `pending_ops`
-    let op = if isolate.eager_poll_count != 50 {
-      isolate.eager_poll_count += 1;
-      match op {
-        Op::Async(mut fut) => {
-          // Tries to eagerly poll async ops once. Often they are immediately ready, in
-          // which case they can be turned into a sync op before we return to V8. This
-          // can save a boundary crossing.
-          #[allow(clippy::match_wild_err_arm)]
-          let mut cx = Context::from_waker(futures::task::noop_waker_ref());
-          match fut.poll_unpin(&mut cx) {
-            Poll::Ready(Err(_)) => panic!("unexpected op error"),
-            Poll::Ready(Ok(buf)) => Op::Sync(buf),
-            Poll::Pending => Op::Async(fut),
-          }
-        }
-        Op::Sync(buf) => Op::Sync(buf),
-      }
-    } else {
-      op
-    };
-
     debug_assert_eq!(isolate.shared.size(), 0);
     match op {
       Op::Sync(buf) => {
@@ -703,7 +678,6 @@ impl Future for Isolate {
 
       // Now handle actual ops.
       inner.have_unpolled_ops = false;
-      inner.eager_poll_count = 0;
       #[allow(clippy::match_wild_err_arm)]
       match inner.pending_ops.poll_next_unpin(cx) {
         Poll::Ready(Some(Err(_))) => panic!("unexpected op error"),