From 06f052b35dd6ea19aa1973bc71c2a9b1cf964b34 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 13 Jul 2017 12:29:45 -0700 Subject: Get rid of local window, fix qps worker --- .../transport/chttp2/transport/chttp2_transport.c | 1 - .../ext/transport/chttp2/transport/flow_control.c | 23 +++++++++++----------- src/core/ext/transport/chttp2/transport/internal.h | 17 ++++++---------- test/cpp/microbenchmarks/bm_chttp2_transport.cc | 1 - test/cpp/microbenchmarks/bm_fullstack_trickle.cc | 6 ++---- 5 files changed, 19 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 35998e94e1..059caaa290 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -265,7 +265,6 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, t->endpoint_reading = 1; t->next_stream_id = is_client ? 1 : 2; t->is_client = is_client; - t->local_window = DEFAULT_WINDOW; t->remote_window = DEFAULT_WINDOW; t->announced_window = DEFAULT_WINDOW; t->deframe_state = is_client ? GRPC_DTS_FH_0 : GRPC_DTS_CLIENT_PREFIX_0; diff --git a/src/core/ext/transport/chttp2/transport/flow_control.c b/src/core/ext/transport/chttp2/transport/flow_control.c index 3fd36a5fb8..7de714c7d4 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.c +++ b/src/core/ext/transport/chttp2/transport/flow_control.c @@ -27,11 +27,14 @@ #include "src/core/lib/support/string.h" +static uint32_t grpc_chttp2_target_announced_window( + const grpc_chttp2_transport* t); + #ifndef NDEBUG typedef struct { int64_t remote_window; - int64_t local_window; + int64_t target_window; int64_t announced_window; int64_t remote_window_delta; int64_t local_window_delta; @@ -41,7 +44,7 @@ typedef struct { static void pretrace(shadow_flow_control* sfc, grpc_chttp2_transport* t, grpc_chttp2_stream* s) { sfc->remote_window = t->remote_window; - sfc->local_window = t->local_window; + sfc->target_window = grpc_chttp2_target_announced_window(t); sfc->announced_window = t->announced_window; if (s != NULL) { sfc->remote_window_delta = s->remote_window_delta; @@ -69,7 +72,8 @@ static void posttrace(shadow_flow_control* sfc, grpc_chttp2_transport* t, uint32_t remote_window = t->settings[GRPC_PEER_SETTINGS][GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE]; char* trw_str = fmt_str(sfc->remote_window, t->remote_window); - char* tlw_str = fmt_str(sfc->local_window, t->local_window); + char* tlw_str = + fmt_str(sfc->target_window, grpc_chttp2_target_announced_window(t)); char* taw_str = fmt_str(sfc->announced_window, t->announced_window); char* srw_str; char* slw_str; @@ -87,7 +91,7 @@ static void posttrace(shadow_flow_control* sfc, grpc_chttp2_transport* t, saw_str = gpr_leftpad("", ' ', 30); } gpr_log(GPR_DEBUG, - "%p[%u][%s] | %s | trw:%s, tlw:%s, taw:%s, srw:%s, slw:%s, saw:%s", t, + "%p[%u][%s] | %s | trw:%s, ttw:%s, taw:%s, srw:%s, slw:%s, saw:%s", t, s != NULL ? s->id : 0, t->is_client ? "cli" : "svr", reason, trw_str, tlw_str, taw_str, srw_str, slw_str, saw_str); gpr_free(trw_str); @@ -177,10 +181,10 @@ grpc_error* grpc_chttp2_flowctl_recv_data(grpc_chttp2_transport* t, grpc_chttp2_stream* s, int64_t incoming_frame_size) { PRETRACE(t, s); - if (incoming_frame_size > t->local_window) { + if (incoming_frame_size > t->announced_window) { char* msg; gpr_asprintf(&msg, "frame of size %d overflows local window of %" PRId64, - t->incoming_frame_size, t->local_window); + t->incoming_frame_size, t->announced_window); grpc_error* err = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); gpr_free(msg); return err; @@ -228,7 +232,6 @@ grpc_error* grpc_chttp2_flowctl_recv_data(grpc_chttp2_transport* t, } t->announced_window -= incoming_frame_size; - t->local_window -= incoming_frame_size; POSTTRACE(t, s, " data recv"); return GRPC_ERROR_NONE; @@ -243,14 +246,11 @@ uint32_t grpc_chttp2_flowctl_maybe_send_transport_update( uint32_t threshold_to_send_transport_window_update = t->outbuf.count > 0 ? 3 * target_announced_window / 4 : target_announced_window / 2; - if (t->announced_window < t->local_window && - t->announced_window <= threshold_to_send_transport_window_update && + if (t->announced_window <= threshold_to_send_transport_window_update && t->announced_window != target_announced_window) { uint32_t announce = (uint32_t)GPR_CLAMP( target_announced_window - t->announced_window, 0, UINT32_MAX); t->announced_window += announce; - t->local_window = - t->announced_window; // announced should never be higher than local. POSTTRACE(t, NULL, "t updt sent"); return announce; } @@ -323,7 +323,6 @@ void grpc_chttp2_flowctl_incoming_bs_update(grpc_chttp2_transport* t, uint32_t add_max_recv_bytes = (uint32_t)(max_recv_bytes - s->local_window_delta); s->local_window_delta += add_max_recv_bytes; - s->t->local_window += add_max_recv_bytes; } POSTTRACE(t, s, "app st recv"); } diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index cb944dce37..fee4469a3a 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -337,17 +337,7 @@ struct grpc_chttp2_transport { /** Our bookkeeping for the remote peer's available window */ int64_t remote_window; - /** Our bookkeeping for our window. Essentially this tracks available buffer - * space to hold data that peer sends to us. This is our local view of the - * window. It does not reflect how the remote peer sees it. */ - int64_t local_window; - - /** This is out window according to what we have sent to our remote peer. The - * difference between this and local_window is what we use to decide when - * to send WINDOW_UPDATE frames. */ - int64_t announced_window; - - /** calculating what we should give for incoming window: + /** calculating what we should give for local window: we track the total amount of flow control over initial window size across all streams: this is data that we want to receive right now (it has an outstanding read) @@ -358,6 +348,11 @@ struct grpc_chttp2_transport { int64_t announced_stream_total_over_incoming_window; int64_t announced_stream_total_under_incoming_window; + /** This is out window according to what we have sent to our remote peer. The + * difference between this and target window is what we use to decide when + * to send WINDOW_UPDATE frames. */ + int64_t announced_window; + /* bdp estimation */ grpc_bdp_estimator bdp_estimator; diff --git a/test/cpp/microbenchmarks/bm_chttp2_transport.cc b/test/cpp/microbenchmarks/bm_chttp2_transport.cc index 3b42c82de2..9193330f78 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_transport.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_transport.cc @@ -522,7 +522,6 @@ static void BM_TransportStreamRecv(benchmark::State &state) { if (!state.KeepRunning()) return; // force outgoing window to be yuge s.chttp2_stream()->local_window_delta = 1024 * 1024 * 1024; - f.chttp2_transport()->local_window = 1024 * 1024 * 1024; s.chttp2_stream()->announced_window_delta = 1024 * 1024 * 1024; f.chttp2_transport()->announced_window = 1024 * 1024 * 1024; received = 0; diff --git a/test/cpp/microbenchmarks/bm_fullstack_trickle.cc b/test/cpp/microbenchmarks/bm_fullstack_trickle.cc index 3464999bf0..13c96ef3e1 100644 --- a/test/cpp/microbenchmarks/bm_fullstack_trickle.cc +++ b/test/cpp/microbenchmarks/bm_fullstack_trickle.cc @@ -74,8 +74,7 @@ class TrickledCHTTP2 : public EndpointPairFixture { write_csv(log_.get(), "t", "iteration", "client_backlog", "server_backlog", "client_t_stall", "client_s_stall", "server_t_stall", "server_s_stall", "client_t_remote", - "server_t_remote", "client_t_local", "server_t_local", - "client_t_announced", "server_t_announced", + "server_t_remote", "client_t_announced", "server_t_announced", "client_s_remote_delta", "server_s_remote_delta", "client_s_local_delta", "server_s_local_delta", "client_s_announced_delta", "server_s_announced_delta", @@ -128,8 +127,7 @@ class TrickledCHTTP2 : public EndpointPairFixture { client->lists[GRPC_CHTTP2_LIST_STALLED_BY_STREAM].head != nullptr, server->lists[GRPC_CHTTP2_LIST_STALLED_BY_TRANSPORT].head != nullptr, server->lists[GRPC_CHTTP2_LIST_STALLED_BY_STREAM].head != nullptr, - client->remote_window, server->remote_window, client->local_window, - server->local_window, client->announced_window, + client->remote_window, server->remote_window, client->announced_window, server->announced_window, client_stream ? client_stream->remote_window_delta : -1, server_stream ? server_stream->remote_window_delta : -1, -- cgit v1.2.3