From 22f00acecbcd329c2706f0721533516b1772eccb Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Thu, 9 Aug 2018 12:13:02 -0700 Subject: [PATCH] add comments to transport --- .../chttp2/transport/chttp2_transport.cc | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 9ad271753ce..983dfa3c050 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -812,6 +812,12 @@ static void set_write_state(grpc_chttp2_transport* t, write_state_name(t->write_state), write_state_name(st), reason)); t->write_state = st; + // If the state is being reset back to idle, it means a write was just + // finished. Make sure all the run_after_write closures are scheduled. + // + // This is also our chance to close the transport if the transport was marked + // to be closed after all writes finish (for example, we received a go-away + // from peer while we had some pending writes) if (st == GRPC_CHTTP2_WRITE_STATE_IDLE) { grpc_chttp2_stream* s; while (grpc_chttp2_list_pop_waiting_for_write_stream(t, &s)) { @@ -903,6 +909,22 @@ void grpc_chttp2_initiate_write(grpc_chttp2_transport* t, grpc_chttp2_initiate_write_reason_string(reason)); t->is_first_write_in_batch = true; GRPC_CHTTP2_REF_TRANSPORT(t, "writing"); + /* Note that the 'write_action_begin_locked' closure is being scheduled + * on the 'finally_scheduler' of t->combiner. This means that + * 'write_action_begin_locked' is called only *after* all the other + * closures (some of which are potentially initiating more writes on the + * transport) are executed on the t->combiner. + * + * The reason for scheduling on finally_scheduler is to make sure we batch + * as many writes as possible. 'write_action_begin_locked' is the function + * that gathers all the relevant bytes (which are at various places in the + * grpc_chttp2_transport structure) and append them to 'outbuf' field in + * grpc_chttp2_transport thereby batching what would have been potentially + * multiple write operations. + * + * Also, 'write_action_begin_locked' only gathers the bytes into outbuf. + * It does not call the endpoint to write the bytes. That is done by the + * 'write_action' (which is scheduled by 'write_action_begin_locked') */ GRPC_CLOSURE_SCHED( GRPC_CLOSURE_INIT(&t->write_action_begin_locked, write_action_begin_locked, t, @@ -1014,6 +1036,7 @@ static void write_action(void* gt, grpc_error* error) { grpc_combiner_scheduler(t->combiner))); } +/* Callback from the grpc_endpoint after bytes have been written on the wire */ static void write_action_end_locked(void* tp, grpc_error* error) { GPR_TIMER_SCOPE("terminate_writing_with_lock", 0); grpc_chttp2_transport* t = static_cast(tp);