From 08cefa0408a55982d83413635ea1f1fd60e56f39 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Tue, 20 Jun 2017 11:29:26 -0700 Subject: [PATCH] Don't initiate writes on setting pushes --- .../chttp2/transport/chttp2_transport.c | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 0211169c00e..cd2c6cc4926 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -91,8 +91,9 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *t, static void complete_fetch_locked(grpc_exec_ctx *exec_ctx, void *gs, grpc_error *error); /** Set a transport level setting, and push it to our peer */ -static void push_setting(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, - grpc_chttp2_setting_id id, uint32_t value); +static void queue_setting_update(grpc_exec_ctx *exec_ctx, + grpc_chttp2_transport *t, + grpc_chttp2_setting_id id, uint32_t value); static void close_from_api(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_chttp2_stream *s, grpc_error *error); @@ -342,15 +343,16 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, /* configure http2 the way we like it */ if (is_client) { - push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_ENABLE_PUSH, 0); - push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 0); + queue_setting_update(exec_ctx, t, GRPC_CHTTP2_SETTINGS_ENABLE_PUSH, 0); + queue_setting_update(exec_ctx, t, + GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, 0); } - push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, - DEFAULT_WINDOW); - push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, - DEFAULT_MAX_HEADER_LIST_SIZE); - push_setting(exec_ctx, t, - GRPC_CHTTP2_SETTINGS_GRPC_ALLOW_TRUE_BINARY_METADATA, 1); + 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, + GRPC_CHTTP2_SETTINGS_GRPC_ALLOW_TRUE_BINARY_METADATA, 1); t->ping_policy = (grpc_chttp2_repeated_ping_policy){ .max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA, @@ -517,8 +519,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, int value = grpc_channel_arg_get_integer( &channel_args->args[i], settings_map[j].integer_options); if (value >= 0) { - push_setting(exec_ctx, t, settings_map[j].setting_id, - (uint32_t)value); + queue_setting_update(exec_ctx, t, settings_map[j].setting_id, + (uint32_t)value); } } break; @@ -929,8 +931,11 @@ static void write_action_end_locked(grpc_exec_ctx *exec_ctx, void *tp, GPR_TIMER_END("terminate_writing_with_lock", 0); } -static void push_setting(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, - grpc_chttp2_setting_id id, uint32_t value) { +// Dirties an HTTP2 setting to be sent out next time a writing path occurs. +// If the change needs to occur immediately, manually initiate a write. +static void queue_setting_update(grpc_exec_ctx *exec_ctx, + grpc_chttp2_transport *t, + grpc_chttp2_setting_id id, uint32_t value) { const grpc_chttp2_setting_parameters *sp = &grpc_chttp2_settings_parameters[id]; uint32_t use_value = GPR_CLAMP(value, sp->min_value, sp->max_value); @@ -941,7 +946,6 @@ static void push_setting(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, if (use_value != t->settings[GRPC_LOCAL_SETTINGS][id]) { t->settings[GRPC_LOCAL_SETTINGS][id] = use_value; t->dirtied_local_settings = 1; - grpc_chttp2_initiate_write(exec_ctx, t, "push_setting"); } } @@ -2107,8 +2111,8 @@ static void update_bdp(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, gpr_log(GPR_DEBUG, "%s: update initial window size to %d", t->peer_string, (int)bdp); } - push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, - (uint32_t)bdp); + queue_setting_update(exec_ctx, t, GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, + (uint32_t)bdp); } static void update_frame(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, @@ -2127,8 +2131,8 @@ static void update_frame(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, gpr_log(GPR_DEBUG, "%s: update max_frame size to %d", t->peer_string, (int)frame_size); } - push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE, - (uint32_t)frame_size); + queue_setting_update(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE, + (uint32_t)frame_size); } static grpc_error *try_http_parsing(grpc_exec_ctx *exec_ctx,