From c8027a3b0bf2f659f95cfe00ba269b09f9649cac Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 22 Sep 2017 07:31:48 -0700 Subject: [PATCH] Run pid controller with grpc_millis --- .../chttp2/transport/chttp2_transport.c | 16 +++++++++------- .../transport/chttp2/transport/flow_control.c | 16 +++++++--------- .../ext/transport/chttp2/transport/internal.h | 5 +++-- .../ext/transport/chttp2/transport/parsing.c | 5 +++-- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 4f381199486..4643a867b85 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -563,7 +563,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, } grpc_chttp2_act_on_flowctl_action( - exec_ctx, grpc_chttp2_flowctl_get_action(&t->flow_control, NULL), t, + exec_ctx, + grpc_chttp2_flowctl_get_action(exec_ctx, &t->flow_control, NULL), t, NULL); grpc_chttp2_initiate_write(exec_ctx, t, @@ -1620,8 +1621,8 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op, &t->flow_control, &s->flow_control, GRPC_HEADER_SIZE_IN_BYTES, already_received); grpc_chttp2_act_on_flowctl_action( - exec_ctx, - grpc_chttp2_flowctl_get_action(&t->flow_control, &s->flow_control), + exec_ctx, grpc_chttp2_flowctl_get_action(exec_ctx, &t->flow_control, + &s->flow_control), t, s); } } @@ -2538,7 +2539,8 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_endpoint_read(exec_ctx, t->ep, &t->read_buffer, &t->read_action_locked); grpc_chttp2_act_on_flowctl_action( - exec_ctx, grpc_chttp2_flowctl_get_action(&t->flow_control, NULL), t, + exec_ctx, + grpc_chttp2_flowctl_get_action(exec_ctx, &t->flow_control, NULL), t, NULL); GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "keep_reading"); } else { @@ -2794,9 +2796,9 @@ static void incoming_byte_stream_next_locked(grpc_exec_ctx *exec_ctx, bs->next_action.max_size_hint, cur_length); grpc_chttp2_act_on_flowctl_action( - exec_ctx, - grpc_chttp2_flowctl_get_action(&t->flow_control, &s->flow_control), t, - s); + exec_ctx, grpc_chttp2_flowctl_get_action(exec_ctx, &t->flow_control, + &s->flow_control), + t, s); } GPR_ASSERT(s->unprocessed_incoming_frames_buffer.length == 0); if (s->frame_storage.length > 0) { diff --git a/src/core/ext/transport/chttp2/transport/flow_control.c b/src/core/ext/transport/chttp2/transport/flow_control.c index aa1dd2d3cbe..a95b4b3d052 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.c +++ b/src/core/ext/transport/chttp2/transport/flow_control.c @@ -391,9 +391,10 @@ static grpc_chttp2_flowctl_urgency delta_is_significant( // Takes in a target and uses the pid controller to return a stabilized // guess at the new bdp. -static double get_pid_controller_guess(grpc_chttp2_transport_flowctl* tfc, +static double get_pid_controller_guess(grpc_exec_ctx* exec_ctx, + grpc_chttp2_transport_flowctl* tfc, double target) { - gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); + grpc_millis now = grpc_exec_ctx_now(exec_ctx); if (!tfc->pid_controller_initialized) { tfc->last_pid_update = now; tfc->pid_controller_initialized = true; @@ -409,11 +410,7 @@ static double get_pid_controller_guess(grpc_chttp2_transport_flowctl* tfc, 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) { - dt = 0.1; - } + double dt = (double)(now - tfc->last_pid_update) * 1e-3; double log2_bdp_guess = grpc_pid_controller_update(&tfc->pid_controller, bdp_error, dt); tfc->last_pid_update = now; @@ -441,7 +438,8 @@ static double get_target_under_memory_pressure( } grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action( - grpc_chttp2_transport_flowctl* tfc, grpc_chttp2_stream_flowctl* sfc) { + grpc_exec_ctx* exec_ctx, grpc_chttp2_transport_flowctl* tfc, + grpc_chttp2_stream_flowctl* sfc) { grpc_chttp2_flowctl_action action; memset(&action, 0, sizeof(action)); // TODO(ncteisen): tune this @@ -473,7 +471,7 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action( // run our target through the pid controller to stabilize change. // TODO(ncteisen): experiment with other controllers here. - double bdp_guess = get_pid_controller_guess(tfc, target); + double bdp_guess = get_pid_controller_guess(exec_ctx, tfc, target); // Though initial window 'could' drop to 0, we keep the floor at 128 tfc->target_initial_window_size = diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index b5850564e02..7c890423a66 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -269,7 +269,7 @@ typedef struct { /* pid controller */ bool pid_controller_initialized; grpc_pid_controller pid_controller; - gpr_timespec last_pid_update; + grpc_millis last_pid_update; // pointer back to transport for tracing const grpc_chttp2_transport *t; @@ -748,7 +748,8 @@ typedef struct { // Reads the flow control data and returns and actionable struct that will tell // chttp2 exactly what it needs to do grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action( - grpc_chttp2_transport_flowctl *tfc, grpc_chttp2_stream_flowctl *sfc); + grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_flowctl *tfc, + grpc_chttp2_stream_flowctl *sfc); // Takes in a flow control action and performs all the needed operations. void grpc_chttp2_act_on_flowctl_action(grpc_exec_ctx *exec_ctx, diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index 2086f42cb20..aab11e02d7b 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/src/core/ext/transport/chttp2/transport/parsing.c @@ -359,8 +359,9 @@ static grpc_error *init_data_frame_parser(grpc_exec_ctx *exec_ctx, s == NULL ? NULL : &s->flow_control, t->incoming_frame_size); grpc_chttp2_act_on_flowctl_action( - exec_ctx, grpc_chttp2_flowctl_get_action( - &t->flow_control, s == NULL ? NULL : &s->flow_control), + exec_ctx, + grpc_chttp2_flowctl_get_action(exec_ctx, &t->flow_control, + s == NULL ? NULL : &s->flow_control), t, s); if (err != GRPC_ERROR_NONE) { goto error_handler;