From b867d0c6a303fcb05ac7af8dc2db251204bd2994 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 10 Feb 2016 13:49:25 +0000 Subject: [PATCH] tuning the output passing and flushing a bit git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1729598 13f79535-47bb-0310-9956-ffa450edef68 --- modules/http2/h2_conn_io.c | 62 ++++++++++++++------------------------ modules/http2/h2_conn_io.h | 35 +++++++++++++++++---- modules/http2/h2_session.c | 45 ++++++++++++++++----------- modules/http2/h2_session.h | 1 + 4 files changed, 80 insertions(+), 63 deletions(-) diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c index 1d6d73e088..b8ce2c0eb1 100644 --- a/modules/http2/h2_conn_io.c +++ b/modules/http2/h2_conn_io.c @@ -174,37 +174,12 @@ static apr_status_t bucketeer_buffer(h2_conn_io *io) apr_status_t h2_conn_io_writeb(h2_conn_io *io, apr_bucket *b) { APR_BRIGADE_INSERT_TAIL(io->output, b); - io->unflushed = 1; return APR_SUCCESS; } -apr_status_t h2_conn_io_consider_flush(h2_conn_io *io) -{ - apr_status_t status = APR_SUCCESS; - - /* The HTTP/1.1 network output buffer/flush behaviour does not - * give optimal performance in the HTTP/2 case, as the pattern of - * buckets (data/eor/eos) is different. - * As long as we have not found out the "best" way to deal with - * this, force a flush at least every WRITE_BUFFER_SIZE amount - * of data. - */ - if (io->unflushed) { - apr_off_t len = 0; - if (!APR_BRIGADE_EMPTY(io->output)) { - apr_brigade_length(io->output, 0, &len); - } - len += io->buflen; - if (len >= WRITE_BUFFER_SIZE) { - return h2_conn_io_pass(io); - } - } - return status; -} - static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force, int eoc) { - if (io->unflushed || force) { + if (io->buflen > 0 || !APR_BRIGADE_EMPTY(io->output)) { pass_out_ctx ctx; if (io->buflen > 0) { @@ -212,7 +187,6 @@ static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force, int eoc) ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection, "h2_conn_io: flush, flushing %ld bytes", (long)io->buflen); bucketeer_buffer(io); - io->buflen = 0; } if (force) { @@ -223,10 +197,10 @@ static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force, int eoc) ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection, "h2_conn_io: flush"); /* Send it out */ - io->unflushed = 0; - + io->buflen = 0; ctx.c = io->connection; ctx.io = eoc? NULL : io; + return pass_out(io->output, &ctx); /* no more access after this, as we might have flushed an EOC bucket * that de-allocated us all. */ @@ -234,20 +208,32 @@ static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force, int eoc) return APR_SUCCESS; } -apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, apr_bucket *b) +apr_status_t h2_conn_io_pass(h2_conn_io *io, int flush) { - APR_BRIGADE_INSERT_TAIL(io->output, b); - return h2_conn_io_flush_int(io, 1, 1); + return h2_conn_io_flush_int(io, flush, 0); } -apr_status_t h2_conn_io_flush(h2_conn_io *io) +apr_status_t h2_conn_io_consider_pass(h2_conn_io *io) { - return h2_conn_io_flush_int(io, 1, 0); + apr_off_t len = 0; + + if (!APR_BRIGADE_EMPTY(io->output)) { + apr_brigade_length(io->output, 0, &len); + } + len += io->buflen; + if (len >= WRITE_BUFFER_SIZE) { + return h2_conn_io_pass(io, 0); + } + return APR_SUCCESS; } -apr_status_t h2_conn_io_pass(h2_conn_io *io) +apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, h2_session *session) { - return h2_conn_io_flush_int(io, 0, 0); + apr_bucket *b = h2_bucket_eoc_create(io->connection->bucket_alloc, session); + APR_BRIGADE_INSERT_TAIL(io->output, b); + b = apr_bucket_flush_create(io->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(io->output, b); + return h2_conn_io_flush_int(io, 0, 1); } apr_status_t h2_conn_io_write(h2_conn_io *io, @@ -258,14 +244,12 @@ apr_status_t h2_conn_io_write(h2_conn_io *io, ctx.c = io->connection; ctx.io = io; - io->unflushed = 1; if (io->bufsize > 0) { ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection, "h2_conn_io: buffering %ld bytes", (long)length); if (!APR_BRIGADE_EMPTY(io->output)) { - status = h2_conn_io_pass(io); - io->unflushed = 1; + status = h2_conn_io_flush_int(io, 0, 0); } while (length > 0 && (status == APR_SUCCESS)) { diff --git a/modules/http2/h2_conn_io.h b/modules/http2/h2_conn_io.h index 15457eb3b6..f0243927d6 100644 --- a/modules/http2/h2_conn_io.h +++ b/modules/http2/h2_conn_io.h @@ -42,7 +42,6 @@ typedef struct { char *buffer; apr_size_t buflen; apr_size_t bufsize; - int unflushed; } h2_conn_io; apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c, @@ -51,16 +50,40 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c, int h2_conn_io_is_buffered(h2_conn_io *io); +/** + * Append data to the buffered output. + * @param buf the data to append + * @param length the length of the data to append + */ apr_status_t h2_conn_io_write(h2_conn_io *io, const char *buf, size_t length); - + +/** + * Append a bucket to the buffered output. + * @param io the connection io + * @param b the bucket to append + */ apr_status_t h2_conn_io_writeb(h2_conn_io *io, apr_bucket *b); -apr_status_t h2_conn_io_consider_flush(h2_conn_io *io); +/** + * Append an End-Of-Connection bucket to the output that, once destroyed, + * will tear down the complete http2 session. + */ +apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, struct h2_session *session); -apr_status_t h2_conn_io_pass(h2_conn_io *io); -apr_status_t h2_conn_io_flush(h2_conn_io *io); -apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, apr_bucket *b); +/** + * Pass any buffered data on to the connection output filters. + * @param io the connection io + * @param flush if a flush bucket should be appended to any output + */ +apr_status_t h2_conn_io_pass(h2_conn_io *io, int flush); + +/** + * Check the amount of buffered output and pass it on if enough has accumulated. + * @param io the connection io + * @param flush if a flush bucket should be appended to any output + */ +apr_status_t h2_conn_io_consider_pass(h2_conn_io *io); #endif /* defined(__mod_h2__h2_conn_io__) */ diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 8bac3e3554..0d94e05aae 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -584,7 +584,7 @@ static int on_send_data_cb(nghttp2_session *ngh2, if (status == APR_SUCCESS) { stream->data_frames_sent++; - h2_conn_io_consider_flush(&session->io); + h2_conn_io_consider_pass(&session->io); return 0; } else { @@ -612,6 +612,15 @@ static int on_frame_send_cb(nghttp2_session *ngh2, (long)session->frames_sent); } ++session->frames_sent; + switch (frame->hd.type) { + case NGHTTP2_HEADERS: + case NGHTTP2_DATA: + /* no explicit flushing necessary */ + break; + default: + session->flush = 1; + break; + } return 0; } @@ -702,7 +711,7 @@ static apr_status_t h2_session_shutdown(h2_session *session, int reason, h2_mplx_get_max_stream_started(session->mplx), reason, (uint8_t*)err, err? strlen(err):0); status = nghttp2_session_send(session->ngh2); - h2_conn_io_flush(&session->io); + h2_conn_io_pass(&session->io, 1); ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03069) "session(%ld): sent GOAWAY, err=%d, msg=%s", session->id, reason, err? err : ""); @@ -1027,6 +1036,8 @@ static apr_status_t h2_session_start(h2_session *session, int *rv) nghttp2_strerror(*rv)); } } + + h2_conn_io_pass(&session->io, 1); return status; } @@ -1859,7 +1870,6 @@ static void h2_session_ev_pre_close(h2_session *session, int arg, const char *ms break; default: h2_session_shutdown(session, arg, msg, 1); - h2_conn_io_flush(&session->io); break; } } @@ -1993,10 +2003,13 @@ apr_status_t h2_session_process(h2_session *session, int async) * time here for the next frame to arrive, before handing * it to keep_alive processing of the mpm. */ - h2_filter_cin_timeout_set(session->cin, - no_streams? apr_time_from_msec(200) - : session->s->timeout); - status = h2_session_read(session, 1, 10); + if (no_streams) { + status = h2_session_read(session, 0, 20); + } + else { + h2_filter_cin_timeout_set(session->cin, session->s->timeout); + status = h2_session_read(session, 1, 20); + } if (status == APR_SUCCESS) { have_read = 1; @@ -2048,7 +2061,7 @@ apr_status_t h2_session_process(h2_session *session, int async) case H2_SESSION_ST_REMOTE_SHUTDOWN: if (nghttp2_session_want_read(session->ngh2)) { h2_filter_cin_timeout_set(session->cin, session->s->timeout); - status = h2_session_read(session, 0, 10); + status = h2_session_read(session, 0, 20); if (status == APR_SUCCESS) { have_read = 1; dispatch_event(session, H2_SESSION_EV_DATA_READ, 0, NULL); @@ -2087,7 +2100,7 @@ apr_status_t h2_session_process(h2_session *session, int async) } } - if (nghttp2_session_want_write(session->ngh2)) { + while (nghttp2_session_want_write(session->ngh2)) { ap_update_child_status(session->c->sbh, SERVER_BUSY_WRITE, NULL); status = h2_session_send(session); if (status == APR_SUCCESS) { @@ -2161,19 +2174,16 @@ apr_status_t h2_session_process(h2_session *session, int async) break; } - if (have_written) { - h2_conn_io_flush(&session->io); - } - else if (!nghttp2_session_want_read(session->ngh2) + h2_conn_io_pass(&session->io, 1); + if (!nghttp2_session_want_read(session->ngh2) && !nghttp2_session_want_write(session->ngh2)) { dispatch_event(session, H2_SESSION_EV_NGH2_DONE, 0, NULL); } } out: - if (have_written) { - h2_conn_io_flush(&session->io); - } + h2_conn_io_pass(&session->io, session->flush); + session->flush = 0; ap_log_cerror( APLOG_MARK, APLOG_TRACE1, status, c, "h2_session(%ld): [%s] process returns", @@ -2190,8 +2200,7 @@ out: if (session->state == H2_SESSION_ST_DONE) { if (!session->eoc_written) { session->eoc_written = 1; - h2_conn_io_write_eoc(&session->io, - h2_bucket_eoc_create(session->c->bucket_alloc, session)); + h2_conn_io_write_eoc(&session->io, session); } } diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h index 5bc1d937bd..118be3c954 100644 --- a/modules/http2/h2_session.h +++ b/modules/http2/h2_session.h @@ -83,6 +83,7 @@ typedef struct h2_session { h2_session_state state; /* state session is in */ unsigned int reprioritize : 1; /* scheduled streams priority changed */ unsigned int eoc_written : 1; /* h2 eoc bucket written */ + unsigned int flush : 1; /* flushing output necessary */ apr_interval_time_t wait_us; /* timout during BUSY_WAIT state, micro secs */ int unsent_submits; /* number of submitted, but not yet written responses. */ -- 2.49.0