]> granicus.if.org Git - apache/commitdiff
removed HackMpm and MaxWrite config directives, added dynamic write size behaviour...
authorStefan Eissing <icing@apache.org>
Fri, 14 Aug 2015 14:07:43 +0000 (14:07 +0000)
committerStefan Eissing <icing@apache.org>
Fri, 14 Aug 2015 14:07:43 +0000 (14:07 +0000)
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1695920 13f79535-47bb-0310-9956-ffa450edef68

docs/manual/mod/mod_h2.xml
modules/http2/h2_config.c
modules/http2/h2_config.h
modules/http2/h2_conn.c
modules/http2/h2_conn_io.c
modules/http2/h2_conn_io.h

index 47a0e99c566fcbec4bdccc0c712b610b961dc370..47d8a3a702c981b31115f43695b2db18bc6ce8c4 100644 (file)
         </usage>
     </directivesynopsis>
 
-    <directivesynopsis>
-        <name>H2BufferWriteMax</name>
-        <description>Maximum size of write on a HTTP/2 connection.</description>
-        <syntax>H2BufferWriteMax <em>bytes</em></syntax>
-        <default>H2BufferWriteMax 16384</default>
-        <contextlist>
-            <context>server config</context>
-            <context>virtual host</context>
-        </contextlist>
-        <usage>
-            <p>
-                This directive sets maximum amount of data sent out in a single 
-                write on a http/2 connection. It only takes effect when 
-                <code>H2BufferOutput</code> is switched on.
-            </p><p>
-                This directive affects performance of underlying TLS transports. TLS
-                transforms each write into an encrypted record. Clients need
-                to receive all of the record in order to decrypt it. Larger sizes
-                result in better server performance, shorter sizes can affect web
-                page paint timings.
-            </p><p>
-                <code>BufferSize</code> should be a multiple of <code>H2BufferWriteMax</code>.
-                <code>H2BufferWriteMax</code>, if larger than 16k, should be a multiple of 16k,
-                since this is the TLS max record size. Be aware that there are TLS 
-                extensions to limit the record size to powers of 2 less than 16k.
-            </p>
-            <example><title>Example</title>
-                <highlight language="config">
-                    H2BufferWriteMax 8000
-                </highlight>
-            </example>
-        </usage>
-    </directivesynopsis>
-
     <directivesynopsis>
         <name>H2SessionExtraFiles</name>
         <description>Number of Extra File Handles</description>
index 4a589f3243c42b276877fdbdbb68943d9e023572..8bb267ec6f58814049a0e8da30726cbe713e0922 100644 (file)
@@ -46,11 +46,9 @@ static h2_config defconf = {
     NULL,             /* no alt-svcs */
     -1,               /* alt-svc max age */
     0,                /* serialize headers */
-    1,                /* hack mpm event */
     1,                /* h2 direct mode */
     -1,               /* buffer output, by default only for TLS */
     64*1024,          /* buffer size */
-    16*1024,          /* out write max */
     5,                /* # session extra files */
 };
 
@@ -76,12 +74,9 @@ static void *h2_config_create(apr_pool_t *pool,
     conf->stream_max_mem_size  = DEF_VAL;
     conf->alt_svc_max_age      = DEF_VAL;
     conf->serialize_headers    = DEF_VAL;
-    conf->hack_mpm_event       = DEF_VAL;
     conf->h2_direct            = DEF_VAL;
     conf->buffer_output        = DEF_VAL;
-    conf->buffer_output        = DEF_VAL;
     conf->buffer_size          = DEF_VAL;
-    conf->write_max            = DEF_VAL;
     conf->session_extra_files  = DEF_VAL;
     return conf;
 }
@@ -121,11 +116,9 @@ void *h2_config_merge(apr_pool_t *pool, void *basev, void *addv)
     n->alt_svcs = add->alt_svcs? add->alt_svcs : base->alt_svcs;
     n->alt_svc_max_age = H2_CONFIG_GET(add, base, alt_svc_max_age);
     n->serialize_headers = H2_CONFIG_GET(add, base, serialize_headers);
-    n->hack_mpm_event = H2_CONFIG_GET(add, base, hack_mpm_event);
     n->h2_direct      = H2_CONFIG_GET(add, base, h2_direct);
     n->buffer_output  = H2_CONFIG_GET(add, base, buffer_output);
     n->buffer_size    = H2_CONFIG_GET(add, base, buffer_size);
-    n->write_max      = H2_CONFIG_GET(add, base, write_max);
     n->session_extra_files = H2_CONFIG_GET(add, base, session_extra_files);
     
     return n;
@@ -152,16 +145,12 @@ int h2_config_geti(h2_config *conf, h2_config_var_t var)
             return H2_CONFIG_GET(conf, &defconf, alt_svc_max_age);
         case H2_CONF_SER_HEADERS:
             return H2_CONFIG_GET(conf, &defconf, serialize_headers);
-        case H2_CONF_HACK_MPM_EVENT:
-            return H2_CONFIG_GET(conf, &defconf, hack_mpm_event);
         case H2_CONF_DIRECT:
             return H2_CONFIG_GET(conf, &defconf, h2_direct);
         case H2_CONF_BUFFER_OUTPUT:
             return H2_CONFIG_GET(conf, &defconf, buffer_output);
         case H2_CONF_BUFFER_SIZE:
             return H2_CONFIG_GET(conf, &defconf, buffer_size);
-        case H2_CONF_WRITE_MAX:
-            return H2_CONFIG_GET(conf, &defconf, write_max);
         case H2_CONF_SESSION_FILES:
             return H2_CONFIG_GET(conf, &defconf, session_extra_files);
         default:
@@ -304,22 +293,6 @@ static const char *h2_conf_set_buffer_size(cmd_parms *parms,
     return NULL;
 }
 
-static const char *h2_conf_set_write_max(cmd_parms *parms,
-                                             void *arg, const char *value)
-{
-    h2_config *cfg = h2_config_sget(parms->server);
-    apr_int64_t max = (int)apr_atoi64(value);
-    if (max <= 0) {
-        return "value must be a positive number";
-    }
-    else if (max > cfg->buffer_size) {
-        return "value must be less than H2BufferSize";
-    }
-    cfg->write_max = (int)max;
-    (void)arg;
-    return NULL;
-}
-
 static const char *h2_conf_set_session_extra_files(cmd_parms *parms,
                                                    void *arg, const char *value)
 {
@@ -350,23 +323,6 @@ static const char *h2_conf_set_serialize_headers(cmd_parms *parms,
     return "value must be On or Off";
 }
 
-static const char *h2_conf_set_hack_mpm_event(cmd_parms *parms,
-                                              void *arg, const char *value)
-{
-    h2_config *cfg = h2_config_sget(parms->server);
-    if (!strcasecmp(value, "On")) {
-        cfg->hack_mpm_event = 1;
-        return NULL;
-    }
-    else if (!strcasecmp(value, "Off")) {
-        cfg->hack_mpm_event = 0;
-        return NULL;
-    }
-    
-    (void)arg;
-    return "value must be On or Off";
-}
-
 static const char *h2_conf_set_direct(cmd_parms *parms,
                                       void *arg, const char *value)
 {
@@ -423,16 +379,12 @@ const command_rec h2_cmds[] = {
                   RSRC_CONF, "set the maximum age (in seconds) that client can rely on alt-svc information"),
     AP_INIT_TAKE1("H2SerializeHeaders", h2_conf_set_serialize_headers, NULL,
                   RSRC_CONF, "on to enable header serialization for compatibility"),
-    AP_INIT_TAKE1("H2HackMpmEvent", h2_conf_set_hack_mpm_event, NULL,
-                  RSRC_CONF, "on to enable a hack that makes mpm_event working with mod_h2"),
     AP_INIT_TAKE1("H2Direct", h2_conf_set_direct, NULL,
                   RSRC_CONF, "on to enable direct HTTP/2 mode"),
     AP_INIT_TAKE1("H2BufferOutput", h2_conf_set_buffer_output, NULL,
                   RSRC_CONF, "on to enable output buffering, default for TLS"),
     AP_INIT_TAKE1("H2BufferSize", h2_conf_set_buffer_size, NULL,
                   RSRC_CONF, "size of outgoing buffer in bytes"),
-    AP_INIT_TAKE1("H2BufferWriteMax", h2_conf_set_write_max, NULL,
-                  RSRC_CONF, "maximum number of bytes in a outgoing write"),
     AP_INIT_TAKE1("H2SessionExtraFiles", h2_conf_set_session_extra_files, NULL,
                   RSRC_CONF, "number of extra file a session might keep open"),
     { NULL, NULL, NULL, 0, 0, NULL }
index 2de6f876ca8d083dd7e0a68191dea30d56fc242c..9bd7ea95d620a779569cb387661c1a98459c704c 100644 (file)
@@ -33,11 +33,9 @@ typedef enum {
     H2_CONF_ALT_SVCS,
     H2_CONF_ALT_SVC_MAX_AGE,
     H2_CONF_SER_HEADERS,
-    H2_CONF_HACK_MPM_EVENT,
     H2_CONF_DIRECT,
     H2_CONF_BUFFER_OUTPUT,
     H2_CONF_BUFFER_SIZE,
-    H2_CONF_WRITE_MAX,
     H2_CONF_SESSION_FILES,
 } h2_config_var_t;
 
@@ -55,12 +53,9 @@ typedef struct h2_config {
     int alt_svc_max_age;          /* seconds clients can rely on alt-svc info*/
     int serialize_headers;        /* Use serialized HTTP/1.1 headers for 
                                      processing, better compatibility */
-    int hack_mpm_event;           /* If mpm_event is detected, perform a hack
-                                     on stream connections to make it work */
     int h2_direct;                /* if mod_h2 is active directly */
     int buffer_output;            /* if output buffering shall be used */  
     int buffer_size;              /* size of buffer for outgoing data */  
-    int write_max;                /* max number of bytes for a write op */  
     int session_extra_files;      /* # of extra files a session may keep open */  
 } h2_config;
 
index 5d5e717080c5fe42892db50826b376efdbedbf48..a37276238e84ff1acc3ffc3b695fb31f27a995ad 100644 (file)
@@ -412,9 +412,7 @@ apr_status_t h2_conn_prep(h2_task_env *env, conn_rec *master, h2_worker *worker)
             /* all fine */
             break;
         case H2_MPM_EVENT: 
-            if (h2_config_geti(cfg, H2_CONF_HACK_MPM_EVENT)) {
-                fix_event_conn(&env->c, master);
-            }
+            fix_event_conn(&env->c, master);
             break;
         default:
             /* fingers crossed */
@@ -471,9 +469,7 @@ apr_status_t h2_conn_init(struct h2_task_env *env, struct h2_worker *worker)
             /* all fine */
             break;
         case H2_MPM_EVENT: 
-            if (h2_config_geti(cfg, H2_CONF_HACK_MPM_EVENT)) {
-                fix_event_conn(&env->c, master);
-            }
+            fix_event_conn(&env->c, master);
             break;
         default:
             /* fingers crossed */
index f876a1f95a4f0f673467405fd6c8d625fa6bda55..749965e1fb6d0c629759edc5e2ff9128a1eb6ddc 100644 (file)
 #include "h2_h2.h"
 #include "h2_util.h"
 
+#define WRITE_SIZE_INITIAL    1300
+#define WRITE_SIZE_MAX        (16*1024)
+#define WRITE_SIZE_IDLE_USEC  (1*APR_USEC_PER_SEC)
+#define WRITE_SIZE_THRESHOLD  (1*1024*1024)
+
 apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c)
 {
     h2_config *cfg = h2_config_get(c);
@@ -36,7 +41,10 @@ apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c)
     io->input = apr_brigade_create(c->pool, c->bucket_alloc);
     io->output = apr_brigade_create(c->pool, c->bucket_alloc);
     io->buflen = 0;
-    io->max_write_size = h2_config_geti(cfg, H2_CONF_WRITE_MAX);
+    /* That is where we start with, 
+     * see https://issues.apache.org/jira/browse/TS-2503 */
+    io->write_size = WRITE_SIZE_INITIAL; 
+    io->last_write = 0;
     io->buffer_output = h2_config_geti(cfg, H2_CONF_BUFFER_OUTPUT);
     
     if (io->buffer_output < 0) {
@@ -175,27 +183,52 @@ static apr_status_t flush_out(apr_bucket_brigade *bb, void *ctx)
 {
     h2_conn_io *io = (h2_conn_io*)ctx;
     apr_status_t status;
+    apr_off_t bblen;
     
     ap_update_child_status(io->connection->sbh, SERVER_BUSY_WRITE, NULL);
-    
-    status = ap_pass_brigade(io->connection->output_filters, bb);
-    apr_brigade_cleanup(bb);
+    status = apr_brigade_length(bb, 1, &bblen);
+    if (status == APR_SUCCESS) {
+        status = ap_pass_brigade(io->connection->output_filters, bb);
+        if (status == APR_SUCCESS) {
+            io->bytes_written += (apr_size_t)bblen;
+            io->last_write = apr_time_now();
+        }
+        apr_brigade_cleanup(bb);
+    }
     return status;
 }
 
 static apr_status_t bucketeer_buffer(h2_conn_io *io) {
     const char *data = io->buffer;
     apr_size_t remaining = io->buflen;
-    int bcount = (int)(remaining / io->max_write_size);
     apr_bucket *b;
-    int i;
+    int bcount, i;
+
+    if (io->write_size > WRITE_SIZE_INITIAL
+        && (apr_time_now() - io->last_write) >= WRITE_SIZE_IDLE_USEC) {
+        /* long time not written, reset write size */
+        io->write_size = WRITE_SIZE_INITIAL;
+        io->bytes_written = 0;
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, io->connection,
+                      "h2_conn_io(%ld): timeout write size reset to %ld", 
+                      (long)io->connection->id, (long)io->write_size);
+    }
+    else if (io->write_size < WRITE_SIZE_MAX 
+             && io->bytes_written >= WRITE_SIZE_THRESHOLD) {
+        /* connection is hot, use max size */
+        io->write_size = WRITE_SIZE_MAX;
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, io->connection,
+                      "h2_conn_io(%ld): threshold reached, write size now %ld", 
+                      (long)io->connection->id, (long)io->write_size);
+    }
     
+    bcount = (int)(remaining / io->write_size);
     for (i = 0; i < bcount; ++i) {
-        b = apr_bucket_transient_create(data, io->max_write_size, 
+        b = apr_bucket_transient_create(data, io->write_size, 
                                         io->output->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(io->output, b);
-        data += io->max_write_size;
-        remaining -= io->max_write_size;
+        data += io->write_size;
+        remaining -= io->write_size;
     }
     
     if (remaining > 0) {
@@ -261,12 +294,9 @@ apr_status_t h2_conn_io_flush(h2_conn_io *io)
     if (io->unflushed) {
         apr_status_t status; 
         if (io->buflen > 0) {
-            apr_bucket *b;
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, io->connection,
                           "h2_conn_io: flush, flushing %ld bytes", (long)io->buflen);
-            b = apr_bucket_transient_create(io->buffer, io->buflen, 
-                                                        io->output->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(io->output, b);
+            bucketeer_buffer(io);
             io->buflen = 0;
         }
         /* Append flush.
index 9d1db6c9b4786d0676eb1171e8c399c8ea9ef6f8..084445ef2b00d317b528faca30fd8356e4cd3c7f 100644 (file)
@@ -27,7 +27,9 @@ typedef struct {
     apr_bucket_brigade *input;
     apr_bucket_brigade *output;
     int buffer_output;
-    int max_write_size;
+    int write_size;
+    apr_time_t last_write;
+    apr_size_t bytes_written;
     
     char *buffer;
     apr_size_t buflen;