mod_http2: avoid unnecessary out flushing, avoid scoreboard updates unless code/msg changes

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1734575 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 68 insertions, 64 deletions

diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c
index 84c953e26a..8a6e66784e 100644
--- a/modules/http2/h2_conn_io.c
+++ b/modules/http2/h2_conn_io.c
@@ -43,18 +43,18 @@
  * which seems to create less TCP packets overall
  */
 #define WRITE_SIZE_MAX        (TLS_DATA_MAX - 100) 
-
 #define WRITE_BUFFER_SIZE     (5*WRITE_SIZE_MAX)
 
+
 apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c, 
                              const h2_config *cfg, 
                              apr_pool_t *pool)
 {
-    io->connection         = c;
-    io->output             = apr_brigade_create(pool, c->bucket_alloc);
-    io->buflen             = 0;
-    io->is_tls             = h2_h2_is_tls(c);
-    io->buffer_output      = io->is_tls;
+    io->c             = c;
+    io->output        = apr_brigade_create(pool, c->bucket_alloc);
+    io->buflen        = 0;
+    io->is_tls        = h2_h2_is_tls(c);
+    io->buffer_output = io->is_tls;
     
     if (io->buffer_output) {
         io->bufsize = WRITE_BUFFER_SIZE;
@@ -65,8 +65,9 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c,
     }
     
     if (io->is_tls) {
-        /* That is where we start with, 
-         * see https://issues.apache.org/jira/browse/TS-2503 */
+        /* This is what we start with, 
+         * see https://issues.apache.org/jira/browse/TS-2503 
+         */
         io->warmup_size    = h2_config_geti64(cfg, H2_CONF_TLS_WARMUP_SIZE);
         io->cooldown_usecs = (h2_config_geti(cfg, H2_CONF_TLS_COOLDOWN_SECS) 
                               * APR_USEC_PER_SEC);
@@ -79,9 +80,10 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c,
     }
 
     if (APLOGctrace1(c)) {
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
-                      "h2_conn_io(%ld): init, buffering=%d, warmup_size=%ld, cd_secs=%f",
-                      io->connection->id, io->buffer_output, (long)io->warmup_size,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c,
+                      "h2_conn_io(%ld): init, buffering=%d, warmup_size=%ld, "
+                      "cd_secs=%f", io->c->id, io->buffer_output, 
+                      (long)io->warmup_size,
                       ((float)io->cooldown_usecs/APR_USEC_PER_SEC));
     }
 
@@ -141,17 +143,17 @@ static apr_status_t bucketeer_buffer(h2_conn_io *io)
         /* long time not written, reset write size */
         io->write_size = WRITE_SIZE_INITIAL;
         io->bytes_written = 0;
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c,
                       "h2_conn_io(%ld): timeout write size reset to %ld", 
-                      (long)io->connection->id, (long)io->write_size);
+                      (long)io->c->id, (long)io->write_size);
     }
     else if (io->write_size < WRITE_SIZE_MAX 
              && io->bytes_written >= io->warmup_size) {
         /* connection is hot, use max size */
         io->write_size = WRITE_SIZE_MAX;
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c,
                       "h2_conn_io(%ld): threshold reached, write size now %ld", 
-                      (long)io->connection->id, (long)io->write_size);
+                      (long)io->c->id, (long)io->write_size);
     }
     
     bcount = (int)(remaining / io->write_size);
@@ -179,43 +181,39 @@ apr_status_t h2_conn_io_writeb(h2_conn_io *io, apr_bucket *b)
 
 static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int force, int eoc)
 {
-    if (io->buflen > 0 || !APR_BRIGADE_EMPTY(io->output)) {
-        pass_out_ctx ctx;
-        
-        if (io->buflen > 0) {
-            /* something in the buffer, put it in the output brigade */
-            ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
-                          "h2_conn_io: flush, flushing %ld bytes", (long)io->buflen);
-            bucketeer_buffer(io);
-        }
-        
-        if (force) {
-            APR_BRIGADE_INSERT_TAIL(io->output,
-                                    apr_bucket_flush_create(io->output->bucket_alloc));
-        }
-        
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
-                      "h2_conn_io: flush");
-        /* Send it out */
-        io->buflen = 0;
-        ctx.c = io->connection;
-        ctx.io = eoc? NULL : io;
+    pass_out_ctx ctx;
+    
+    if (io->buflen == 0 && APR_BRIGADE_EMPTY(io->output)) {
+        return APR_SUCCESS;
+    }
         
-        return pass_out(io->output, &ctx);
-        /* no more access after this, as we might have flushed an EOC bucket
-         * that de-allocated us all. */
+    if (io->buflen > 0) {
+        /* something in the buffer, put it in the output brigade */
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c,
+                      "h2_conn_io: flush, flushing %ld bytes", 
+                      (long)io->buflen);
+        bucketeer_buffer(io);
     }
-    return APR_SUCCESS;
+    
+    if (force) {
+        apr_bucket *b = apr_bucket_flush_create(io->c->bucket_alloc);
+        APR_BRIGADE_INSERT_TAIL(io->output, b);
+    }
+    
+    ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c, "h2_conn_io: flush");
+    /* Send it out */
+    io->buflen = 0;
+    ctx.c = io->c;
+    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. */
 }
 
 apr_status_t h2_conn_io_flush(h2_conn_io *io)
 {
-    /* make sure we always write a flush, even if our buffers are empty.
-     * We want to flush not only our buffers, but alse ones further down
-     * the connection filters. */
-    apr_bucket *b = apr_bucket_flush_create(io->connection->bucket_alloc);
-    APR_BRIGADE_INSERT_TAIL(io->output, b);
-    return h2_conn_io_flush_int(io, 0, 0);
+    return h2_conn_io_flush_int(io, 1, 0);
 }
 
 apr_status_t h2_conn_io_consider_pass(h2_conn_io *io)
@@ -234,9 +232,7 @@ apr_status_t h2_conn_io_consider_pass(h2_conn_io *io)
 
 apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, h2_session *session)
 {
-    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_bucket *b = h2_bucket_eoc_create(io->c->bucket_alloc, session);
     APR_BRIGADE_INSERT_TAIL(io->output, b);
     return h2_conn_io_flush_int(io, 0, 1);
 }
@@ -247,10 +243,10 @@ apr_status_t h2_conn_io_write(h2_conn_io *io,
     apr_status_t status = APR_SUCCESS;
     pass_out_ctx ctx;
     
-    ctx.c = io->connection;
+    ctx.c = io->c;
     ctx.io = io;
     if (io->bufsize > 0) {
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->connection,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c,
                       "h2_conn_io: buffering %ld bytes", (long)length);
                       
         if (!APR_BRIGADE_EMPTY(io->output)) {
@@ -278,7 +274,7 @@ apr_status_t h2_conn_io_write(h2_conn_io *io,
         
     }
     else {
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, status, io->connection,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE4, status, io->c,
                       "h2_conn_io: writing %ld bytes to brigade", (long)length);
         status = apr_brigade_write(io->output, pass_out, &ctx, buf, length);
     }
diff --git a/modules/http2/h2_conn_io.h b/modules/http2/h2_conn_io.h
index 9c40ae615f..b8be671d38 100644
--- a/modules/http2/h2_conn_io.h
+++ b/modules/http2/h2_conn_io.h
@@ -26,7 +26,7 @@ struct h2_session;
  * directly without copying.
  */
 typedef struct {
-    conn_rec *connection;
+    conn_rec *c;
     apr_bucket_brigade *output;
 
     int is_tls;
diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index d24ac0e90e..08bcd18b0e 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -1956,15 +1956,20 @@ static const int MAX_WAIT_MICROS = 200 * 1000;
 
 static void update_child_status(h2_session *session, int status, const char *msg)
 {
-    apr_snprintf(session->status, sizeof(session->status),
-                 "%s, streams: %d/%d/%d/%d/%d (open/recv/resp/push/rst)", 
-                 msg? msg : "-",
-                 (int)h2_ihash_count(session->streams), 
-                 (int)session->requests_received,
-                 (int)session->responses_submitted,
-                 (int)session->pushes_submitted,
-                 (int)session->pushes_reset + session->streams_reset);
-    ap_update_child_status_descr(session->c->sbh, status, session->status);
+    /* Assume that we also change code/msg when something really happened and
+     * avoid updating the scoreboard in between */
+    if (session->last_status_code != status 
+        || session->last_status_msg != msg) {
+        apr_snprintf(session->status, sizeof(session->status),
+                     "%s, streams: %d/%d/%d/%d/%d (open/recv/resp/push/rst)", 
+                     msg? msg : "-",
+                     (int)h2_ihash_count(session->streams), 
+                     (int)session->requests_received,
+                     (int)session->responses_submitted,
+                     (int)session->pushes_submitted,
+                     (int)session->pushes_reset + session->streams_reset);
+        ap_update_child_status_descr(session->c->sbh, status, session->status);
+    }
 }
 
 apr_status_t h2_session_process(h2_session *session, int async)
@@ -2094,10 +2099,12 @@ apr_status_t h2_session_process(h2_session *session, int async)
             case H2_SESSION_ST_LOCAL_SHUTDOWN:
             case H2_SESSION_ST_REMOTE_SHUTDOWN:
                 if (nghttp2_session_want_read(session->ngh2)) {
+                    ap_update_child_status(session->c->sbh, SERVER_BUSY_READ, NULL);
                     h2_filter_cin_timeout_set(session->cin, session->s->timeout);
                     status = h2_session_read(session, 0);
                     if (status == APR_SUCCESS) {
                         have_read = 1;
+                        update_child_status(session, SERVER_BUSY_READ, "busy");
                         dispatch_event(session, H2_SESSION_EV_DATA_READ, 0, NULL);
                     }
                     else if (status == APR_EAGAIN) {
@@ -2134,7 +2141,7 @@ apr_status_t h2_session_process(h2_session *session, int async)
                     }
                 }
                 
-                while (nghttp2_session_want_write(session->ngh2)) {
+                if (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) {
@@ -2150,7 +2157,6 @@ apr_status_t h2_session_process(h2_session *session, int async)
                 if (have_read || have_written) {
                     if (session->wait_us) {
                         session->wait_us = 0;
-                        update_child_status(session, SERVER_BUSY_READ, "busy");
                     }
                 }
                 else if (!nghttp2_session_want_write(session->ngh2)) {
diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h
index fa98bf9186..c8e131d114 100644
--- a/modules/http2/h2_session.h
+++ b/modules/http2/h2_session.h
@@ -130,6 +130,8 @@ typedef struct h2_session {
     struct h2_push_diary *push_diary; /* remember pushes, avoid duplicates */
     
     char status[64];                /* status message for scoreboard */
+    int last_status_code;           /* the one already reported */
+    const char *last_status_msg;    /* the one already reported */
 } h2_session;