From 03c049b14f1a674b8223648c3ee027e9904ea03f Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Mon, 3 Oct 2022 12:59:39 -0700
Subject: [PATCH] [chttp2] Fix performance regression for small stream sends
 (#31180)

* [chttp2] Fix performance regression for small stream sends

* fix

* comment

* comment

* fix
---
 .../chttp2/transport/flow_control.cc          | 44 ++++++++++++++++---
 .../transport/chttp2/transport/flow_control.h |  1 +
 .../transport/chttp2/flow_control_test.cc     |  2 +-
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/src/core/ext/transport/chttp2/transport/flow_control.cc b/src/core/ext/transport/chttp2/transport/flow_control.cc
index b33ad173137..a9ff0e4a556 100644
--- a/src/core/ext/transport/chttp2/transport/flow_control.cc
+++ b/src/core/ext/transport/chttp2/transport/flow_control.cc
@@ -279,7 +279,16 @@ void TransportFlowControl::UpdateSetting(
               grpc_chttp2_settings_parameters[id].max_value);
     if (new_desired_value != *desired_value) {
       *desired_value = new_desired_value;
-      (action->*set)(FlowControlAction::Urgency::QUEUE_UPDATE, *desired_value);
+      // Reaching zero can only happen for initial window size, and if it occurs
+      // we really want to wake up writes and ensure all the queued stream
+      // window updates are flushed, since stream flow control operates
+      // differently at zero window size.
+      FlowControlAction::Urgency urgency =
+          FlowControlAction::Urgency::QUEUE_UPDATE;
+      if (new_desired_value == 0) {
+        urgency = FlowControlAction::Urgency::UPDATE_IMMEDIATELY;
+      }
+      (action->*set)(urgency, *desired_value);
     }
   } else {
     int64_t delta = new_desired_value - *desired_value;
@@ -391,13 +400,34 @@ int64_t StreamFlowControl::DesiredAnnounceSize() const {
 FlowControlAction StreamFlowControl::UpdateAction(FlowControlAction action) {
   const int64_t desired_announce_size = DesiredAnnounceSize();
   if (desired_announce_size > 0) {
-    if ((min_progress_size_ > 0 && announced_window_delta_ <= 0) ||
-        desired_announce_size >= 8192) {
-      action.set_send_stream_update(
-          FlowControlAction::Urgency::UPDATE_IMMEDIATELY);
-    } else {
-      action.set_send_stream_update(FlowControlAction::Urgency::QUEUE_UPDATE);
+    FlowControlAction::Urgency urgency =
+        FlowControlAction::Urgency::QUEUE_UPDATE;
+    // Size at which we probably want to wake up and write regardless of whether
+    // we *have* to.
+    // Currently set at half the initial window size or 8kb (whichever is
+    // greater). 8kb means we don't send rapidly unnecessarily when the initial
+    // window size is small.
+    const int64_t hurry_up_size =
+        std::max(static_cast<int64_t>(tfc_->sent_init_window()) / 2,
+                 static_cast<int64_t>(8192));
+    if (desired_announce_size > hurry_up_size) {
+      urgency = FlowControlAction::Urgency::UPDATE_IMMEDIATELY;
+    }
+    // min_progress_size_ > 0 means we have a reader ready to read.
+    if (min_progress_size_ > 0) {
+      // If we're into initial window to receive that data we should wake up and
+      // send an update.
+      if (announced_window_delta_ < 0) {
+        urgency = FlowControlAction::Urgency::UPDATE_IMMEDIATELY;
+      } else if (announced_window_delta_ == 0 &&
+                 tfc_->sent_init_window() == 0) {
+        // Special case when initial window size is zero, meaning that
+        // announced_window_delta cannot become negative (it may already be so
+        // however).
+        urgency = FlowControlAction::Urgency::UPDATE_IMMEDIATELY;
+      }
     }
+    action.set_send_stream_update(urgency);
   }
   return action;
 }
diff --git a/src/core/ext/transport/chttp2/transport/flow_control.h b/src/core/ext/transport/chttp2/transport/flow_control.h
index 4c0f8d3bed3..8f11971d471 100644
--- a/src/core/ext/transport/chttp2/transport/flow_control.h
+++ b/src/core/ext/transport/chttp2/transport/flow_control.h
@@ -236,6 +236,7 @@ class TransportFlowControl final {
   BdpEstimator* bdp_estimator() { return &bdp_estimator_; }
 
   uint32_t acked_init_window() const { return acked_init_window_; }
+  uint32_t sent_init_window() const { return target_initial_window_size_; }
 
   void SetAckedInitialWindow(uint32_t value) { acked_init_window_ = value; }
 
diff --git a/test/core/transport/chttp2/flow_control_test.cc b/test/core/transport/chttp2/flow_control_test.cc
index a4af318bc41..b1319488d62 100644
--- a/test/core/transport/chttp2/flow_control_test.cc
+++ b/test/core/transport/chttp2/flow_control_test.cc
@@ -160,7 +160,7 @@ TEST(FlowControl, GradualReadsUpdate) {
         break;
     }
   }
-  EXPECT_GT(immediate_updates, 0);
+  EXPECT_GE(immediate_updates, 0);
   EXPECT_GT(queued_updates, 0);
   EXPECT_EQ(immediate_updates + queued_updates, 65535);
 }