Make sure initial settings are as we would pick next iteration

3 files changed, 38 insertions, 29 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index b424bd2da5..e1c2265be4 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -274,6 +274,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
   t->is_client = is_client;
   t->flow_control.remote_window = DEFAULT_WINDOW;
   t->flow_control.announced_window = DEFAULT_WINDOW;
+  t->flow_control.target_initial_window_size = DEFAULT_WINDOW;
   t->flow_control.t = t;
   t->deframe_state = is_client ? GRPC_DTS_FH_0 : GRPC_DTS_CLIENT_PREFIX_0;
   t->is_first_frame = true;
@@ -312,16 +313,6 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                     grpc_combiner_scheduler(t->combiner));
 
   grpc_bdp_estimator_init(&t->flow_control.bdp_estimator, t->peer_string);
-  t->flow_control.last_pid_update = gpr_now(GPR_CLOCK_MONOTONIC);
-  grpc_pid_controller_init(
-      &t->flow_control.pid_controller,
-      (grpc_pid_controller_args){.gain_p = 4,
-                                 .gain_i = 8,
-                                 .gain_d = 0,
-                                 .initial_control_value = log2(DEFAULT_WINDOW),
-                                 .min_control_value = -1,
-                                 .max_control_value = 25,
-                                 .integral_range = 10});
 
   grpc_chttp2_goaway_parser_init(&t->goaway_parser);
   grpc_chttp2_hpack_parser_init(exec_ctx, &t->hpack_parser);
@@ -360,8 +351,6 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
     queue_setting_update(exec_ctx, t,
                          GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 0);
   }
-  queue_setting_update(exec_ctx, t, GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE,
-                       DEFAULT_WINDOW);
   queue_setting_update(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE,
                        DEFAULT_MAX_HEADER_LIST_SIZE);
   queue_setting_update(exec_ctx, t,
@@ -591,6 +580,10 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
     t->keepalive_state = GRPC_CHTTP2_KEEPALIVE_STATE_DISABLED;
   }
 
+  grpc_chttp2_act_on_flowctl_action(
+      exec_ctx, grpc_chttp2_flowctl_get_action(&t->flow_control, NULL), t,
+      NULL);
+
   grpc_chttp2_initiate_write(exec_ctx, t,
                              GRPC_CHTTP2_INITIATE_WRITE_INITIAL_WRITE);
   post_benign_reclaimer(exec_ctx, t);
diff --git a/src/core/ext/transport/chttp2/transport/flow_control.c b/src/core/ext/transport/chttp2/transport/flow_control.c
index e3be2e5198..aa1dd2d3cb 100644
--- a/src/core/ext/transport/chttp2/transport/flow_control.c
+++ b/src/core/ext/transport/chttp2/transport/flow_control.c
@@ -175,11 +175,9 @@ static void trace_action(grpc_chttp2_transport_flowctl* tfc,
 /* How many bytes of incoming flow control would we like to advertise */
 static uint32_t grpc_chttp2_target_announced_window(
     const grpc_chttp2_transport_flowctl* tfc) {
-  return (uint32_t)GPR_MIN(
-      (int64_t)((1u << 31) - 1),
-      tfc->announced_stream_total_over_incoming_window +
-          tfc->t->settings[GRPC_SENT_SETTINGS]
-                          [GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE]);
+  return (uint32_t)GPR_MIN((int64_t)((1u << 31) - 1),
+                           tfc->announced_stream_total_over_incoming_window +
+                               tfc->target_initial_window_size);
 }
 
 // we have sent data on the wire, we must track this in our bookkeeping for the
@@ -395,8 +393,22 @@ static grpc_chttp2_flowctl_urgency delta_is_significant(
 // guess at the new bdp.
 static double get_pid_controller_guess(grpc_chttp2_transport_flowctl* tfc,
                                        double target) {
-  double bdp_error = target - grpc_pid_controller_last(&tfc->pid_controller);
   gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
+  if (!tfc->pid_controller_initialized) {
+    tfc->last_pid_update = now;
+    tfc->pid_controller_initialized = true;
+    grpc_pid_controller_init(
+        &tfc->pid_controller,
+        (grpc_pid_controller_args){.gain_p = 4,
+                                   .gain_i = 8,
+                                   .gain_d = 0,
+                                   .initial_control_value = target,
+                                   .min_control_value = -1,
+                                   .max_control_value = 25,
+                                   .integral_range = 10});
+    return pow(2, target);
+  }
+  double bdp_error = target - grpc_pid_controller_last(&tfc->pid_controller);
   gpr_timespec dt_timespec = gpr_time_sub(now, tfc->last_pid_update);
   double dt = (double)dt_timespec.tv_sec + dt_timespec.tv_nsec * 1e-9;
   if (dt > 0.1) {
@@ -415,7 +427,7 @@ static double get_target_under_memory_pressure(
   double memory_pressure = grpc_resource_quota_get_memory_pressure(
       grpc_resource_user_quota(grpc_endpoint_get_resource_user(tfc->t->ep)));
   static const double kLowMemPressure = 0.1;
-  static const double kZeroTarget = 24;
+  static const double kZeroTarget = 22;
   static const double kHighMemPressure = 0.8;
   static const double kMaxMemPressure = 0.9;
   if (memory_pressure < kLowMemPressure && target < kZeroTarget) {
@@ -432,10 +444,6 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action(
     grpc_chttp2_transport_flowctl* tfc, grpc_chttp2_stream_flowctl* sfc) {
   grpc_chttp2_flowctl_action action;
   memset(&action, 0, sizeof(action));
-  uint32_t target_announced_window = grpc_chttp2_target_announced_window(tfc);
-  if (tfc->announced_window < target_announced_window / 2) {
-    action.send_transport_update = GRPC_CHTTP2_FLOWCTL_UPDATE_IMMEDIATELY;
-  }
   // TODO(ncteisen): tune this
   if (sfc != NULL && !sfc->s->read_closed) {
     uint32_t sent_init_window =
@@ -455,7 +463,6 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action(
 
     // get bdp estimate and update initial_window accordingly.
     int64_t estimate = -1;
-    int32_t bdp = -1;
     if (grpc_bdp_estimator_get_estimate(&tfc->bdp_estimator, &estimate)) {
       double target = 1 + log2((double)estimate);
 
@@ -469,14 +476,15 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action(
       double bdp_guess = get_pid_controller_guess(tfc, target);
 
       // Though initial window 'could' drop to 0, we keep the floor at 128
-      bdp = GPR_MAX((int32_t)bdp_guess, 128);
+      tfc->target_initial_window_size =
+          (int32_t)GPR_CLAMP(bdp_guess, 128, INT32_MAX);
 
       grpc_chttp2_flowctl_urgency init_window_update_urgency =
-          delta_is_significant(tfc, bdp,
+          delta_is_significant(tfc, tfc->target_initial_window_size,
                                GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE);
       if (init_window_update_urgency != GRPC_CHTTP2_FLOWCTL_NO_ACTION_NEEDED) {
         action.send_setting_update = init_window_update_urgency;
-        action.initial_window_size = (uint32_t)bdp;
+        action.initial_window_size = (uint32_t)tfc->target_initial_window_size;
       }
     }
 
@@ -485,8 +493,9 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action(
     if (grpc_bdp_estimator_get_bw(&tfc->bdp_estimator, &bw_dbl)) {
       // we target the max of BDP or bandwidth in microseconds.
       int32_t frame_size = (int32_t)GPR_CLAMP(
-          GPR_MAX((int32_t)GPR_CLAMP(bw_dbl, 0, INT_MAX) / 1000, bdp), 16384,
-          16777215);
+          GPR_MAX((int32_t)GPR_CLAMP(bw_dbl, 0, INT_MAX) / 1000,
+                  tfc->target_initial_window_size),
+          16384, 16777215);
       grpc_chttp2_flowctl_urgency frame_size_urgency = delta_is_significant(
           tfc, frame_size, GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE);
       if (frame_size_urgency != GRPC_CHTTP2_FLOWCTL_NO_ACTION_NEEDED) {
@@ -497,6 +506,10 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action(
       }
     }
   }
+  uint32_t target_announced_window = grpc_chttp2_target_announced_window(tfc);
+  if (tfc->announced_window < target_announced_window / 2) {
+    action.send_transport_update = GRPC_CHTTP2_FLOWCTL_UPDATE_IMMEDIATELY;
+  }
   TRACEACTION(tfc, action);
   return action;
 }
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index 1506d61311..4b66f7d6ea 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -258,6 +258,8 @@ typedef struct {
    * to send WINDOW_UPDATE frames. */
   int64_t announced_window;
 
+  int32_t target_initial_window_size;
+
   /** should we probe bdp? */
   bool enable_bdp_probe;
 
@@ -265,6 +267,7 @@ typedef struct {
   grpc_bdp_estimator bdp_estimator;
 
   /* pid controller */
+  bool pid_controller_initialized;
   grpc_pid_controller pid_controller;
   gpr_timespec last_pid_update;