httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject [Patch] mod_ssl write completion
Date Mon, 28 Oct 2013 23:47:01 GMT
Hi all,

I was on the wrong track with regards mod_ssl and the flush-on-eos, the patch below to mod_ssl
echoes a similar strategy the core output filter uses to enter write completion mode.

The idea is that if the brigade to be written contains an EOS bucket but not a flush bucket,
we can safely set aside the buckets and when event is present enter write completion. As soon
as we do see a flush bucket, or if we have previously set aside we write as normal. Because
we only trigger this behaviour when we see an EOS bucket we only set aside once per request,
which should keep RAM use bounded, and leave generators that stream unbounded data unaffected.

This should in theory make most mod_ssl requests behave similarly to normal requests with
respect to async behaviour. A typical response contains a file bucket and eos, which will
be immediately set aside and enter write completion. Proxied responses should enter write
completion when the last bucket is produced, which could be the whole response if ProxyIOBufferSize
is set large enough. Cached responses also contain just a (file or heap) bucket and eos, so
should also benefit. Any content generator that sends the EOS bucket separately from the data
won't benefit, but we can fix any of those separately.

All the SSL tests pass with this patch, but more eyes welcome.

Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c	(revision 1536348)
+++ modules/ssl/ssl_engine_io.c	(working copy)
@@ -104,6 +104,8 @@
     ap_filter_t        *pInputFilter;
     ap_filter_t        *pOutputFilter;
     SSLConnRec         *config;
+    apr_bucket_brigade *bb;    /* Brigade used to setaside before EOS. */
+    apr_pool_t         *deferred_write_pool;
 } ssl_filter_ctx_t;
 
 typedef struct {
@@ -1675,6 +1677,62 @@
         return ssl_io_filter_error(f, bb, status);
     }
 
+    /* We now try and take advantage of write completion in async mpms.
+     * If our brigade contains an EOS bucket and no flush buckets, set
+     * aside the whole brigade and leave early. We will get called again
+     * during write completion, at which point we'll do the actual write.
+     */
+    if (f->c->cs) {
+
+        if (!filter_ctx->bb) {
+            filter_ctx->bb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
+        }
+
+        if (!APR_BRIGADE_EMPTY(filter_ctx->bb)) {
+
+            APR_BRIGADE_PREPEND(bb, filter_ctx->bb);
+
+            f->c->data_in_output_filters--;
+            ap_assert(f->c->data_in_output_filters >= 0);
+
+        }
+        else {
+            apr_bucket *bucket;
+            int found = 0;
+
+            if (filter_ctx->deferred_write_pool) {
+                apr_pool_clear(filter_ctx->deferred_write_pool);
+            }
+
+            /* EOS, but no flush */
+            for (bucket = APR_BRIGADE_FIRST(bb);
+                    bucket != APR_BRIGADE_SENTINEL(bb); bucket =
+                            APR_BUCKET_NEXT(bucket)) {
+                if (APR_BUCKET_IS_EOS(bucket)) {
+                    found = 1;
+                }
+                if (APR_BUCKET_IS_FLUSH(bucket)) {
+                    found = 0;
+                    break;
+                }
+            }
+
+            /* leave early */
+            if (found) {
+                if (!filter_ctx->deferred_write_pool) {
+                    apr_pool_create(&filter_ctx->deferred_write_pool,
+                            f->c->pool);
+                    apr_pool_tag(filter_ctx->deferred_write_pool,
+                            "ssl_deferred_write");
+                }
+                status = ap_save_brigade(f, &(filter_ctx->bb), &bb,
+                        filter_ctx->deferred_write_pool);
+                f->c->data_in_output_filters++;
+                return APR_SUCCESS;
+            }
+        }
+    }
+
     while (!APR_BRIGADE_EMPTY(bb)) {
         apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
 
@@ -1694,10 +1752,7 @@
                  * without creating a new one since it only contains the
                  * EOS bucket.
                  */
-
-                if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {
-                    return status;
-                }
+                status = ap_pass_brigade(f->next, bb);
                 break;
             }
             else {
@@ -1711,9 +1766,7 @@
             /* The EOC bucket indicates connection closure, so SSL
              * shutdown must now be performed.  */
             ssl_filter_io_shutdown(filter_ctx, f->c, 0);
-            if ((status = ap_pass_brigade(f->next, bb)) != APR_SUCCESS) {
-                return status;
-            }
+            status = ap_pass_brigade(f->next, bb);
             break;
         }
         else {
@@ -1989,6 +2042,9 @@
 
     filter_ctx = apr_palloc(c->pool, sizeof(ssl_filter_ctx_t));
 
+    filter_ctx->bb = NULL;
+    filter_ctx->deferred_write_pool = NULL;
+
     filter_ctx->config          = myConnConfig(c);
 
     ap_add_output_filter(ssl_io_coalesce, NULL, r, c);
Index: server/core_filters.c
===================================================================
--- server/core_filters.c	(revision 1536348)
+++ server/core_filters.c	(working copy)
@@ -413,7 +413,8 @@
         else {
             bb = ctx->buffered_bb;
         }
-        c->data_in_output_filters = 0;
+        c->data_in_output_filters--;
+        ap_assert(f->c->data_in_output_filters >= 0);
     }
     else if (new_bb == NULL) {
         return APR_SUCCESS;
@@ -617,7 +618,7 @@
     }
     remove_empty_buckets(bb);
     if (!APR_BRIGADE_EMPTY(bb)) {
-        c->data_in_output_filters = 1;
+        c->data_in_output_filters++;
         if (bb != ctx->buffered_bb) {
             if (!ctx->deferred_write_pool) {
                 apr_pool_create(&ctx->deferred_write_pool, c->pool);

Regards,
Graham
--


Mime
View raw message