From 2c48148ebdcdfb62bc24c387f29d986606dc1bcd Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 19 Sep 2017 14:07:19 -0700 Subject: Make sure initial settings are as we would pick next iteration --- .../transport/chttp2/transport/chttp2_transport.c | 17 +++----- .../ext/transport/chttp2/transport/flow_control.c | 47 ++++++++++++++-------- src/core/ext/transport/chttp2/transport/internal.h | 3 ++ 3 files changed, 38 insertions(+), 29 deletions(-) (limited to 'src/core') 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; -- cgit v1.2.3