util_filter: split pending filters ring in two: input and output ones.

Pending input and output are now maintained separately in respectively
c->pending_input_filters and c->pending_output_filters, which improves
both performances and debug-ability.

Also, struct ap_filter_ring is made opaque, it's only used by util_filter
and this will allow us to later change it e.g. to a dual ring+apr_hash to
avoid quadratic search in ap_filter_prepare_brigade().

MMN major bumped due to the change in conn_rec (this is trunk only code
anyway for now).

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

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 9bfc74576109341e6eff4e7f7af3041a2f28facd..bfd36d5fa57b94cdf210ff9165a7b32fa2205c2e 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -596,15 +596,17 @@
  *                         scoreboard.h.
  * 20180720.6 (2.5.1-dev)  add strict_host_check to core_server_conf
  * 20180720.7 (2.5.1-dev)  worker_share struct re-organized
+ * 20180902.1 (2.5.1-dev)  Split conn_rec pending_filters in two rings,
+ *                         pending_input_filters and pending_output_filters
  *
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20180720
+#define MODULE_MAGIC_NUMBER_MAJOR 20180902
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 7                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/include/httpd.h b/include/httpd.h
index ff620aefeca1c12e5e66ce466d4c4cd77591a3fd..808e3d4caf11e7e7193e312b04cd628744a88592 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1111,6 +1111,9 @@ typedef enum {
     AP_CONN_KEEPALIVE
 } ap_conn_keepalive_e;
 
+/* For struct ap_filter and ap_filter_ring */
+#include "util_filter.h"
+
 /**
  * @brief Structure to store things which are per connection
  */
@@ -1221,8 +1224,10 @@ struct conn_rec {
     /** Array of requests being handled under this connection. */
     apr_array_header_t *requests;
 
-    /** Ring of pending filters (with setaside buckets) */
-    struct ap_filter_ring *pending_filters;
+    /** Ring of pending input filters (with setaside buckets) */
+    struct ap_filter_ring *pending_input_filters;
+    /** Ring of pending output filters (with setaside buckets) */
+    struct ap_filter_ring *pending_output_filters;
 
     /** The minimum level of filter type to allow setaside buckets */
     int async_filter;

diff --git a/include/util_filter.h b/include/util_filter.h
index af9eba9c1872b9d748d132d4f20bcde795ffa586..e863bfe7a197313a09780fbb2d9d1d75b4ddd2bb 100644 (file)
--- a/include/util_filter.h
+++ b/include/util_filter.h
@@ -304,9 +304,9 @@ struct ap_filter_t {
 };
 
 /**
- * @brief The representation of a filters' ring.
+ * @brief The type of a filters' ring (opaque).
  */
-typedef APR_RING_HEAD(ap_filter_ring, ap_filter_t) ap_filter_ring_t;
+typedef struct ap_filter_ring ap_filter_ring_t;
 
 /**
  * Get the current bucket brigade from the next filter on the filter

diff --git a/server/util_filter.c b/server/util_filter.c
index e2952958dab6370effc4b4ea88b3a3e4f41eb954..e2533694550c7d27c2caa088adfc33ed9a7f284a 100644 (file)
--- a/server/util_filter.c
+++ b/server/util_filter.c
@@ -51,6 +51,8 @@
 #undef APLOG_MODULE_INDEX
 #define APLOG_MODULE_INDEX AP_CORE_MODULE_INDEX
 
+APR_RING_HEAD(ap_filter_ring, ap_filter_t);
+
 typedef struct filter_trie_node filter_trie_node;
 
 typedef struct {
@@ -713,7 +715,8 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f, apr_pool_t **p)
 {
     apr_pool_t *pool;
     ap_filter_t *next, *e;
-    ap_filter_t *found = NULL;
+    ap_filter_ring_t **ref, *pendings;
+    int found = 0;
 
     pool = f->r ? f->r->pool : f->c->pool;
     if (p) {
@@ -728,33 +731,40 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f, apr_pool_t **p)
         return DECLINED;
     }
 
-    /* Pending reads/writes must happen in the same order as input/output
-     * filters, so find the first "next" filter already in place and insert
-     * before it, if any, otherwise insert last.
+    if (f->frec->direction == AP_FILTER_INPUT) {
+        ref = &f->c->pending_input_filters;
+    }
+    else {
+        ref = &f->c->pending_output_filters;
+    }
+    pendings = *ref;
+
+    /* Pending reads/writes must happen in the reverse order of the actual
+     * in/output filters (in/outer most first), though we still maintain the
+     * ring in the same "next" order as filters (walking is backward). So find
+     * the first f->next filter already in place and insert before if
+     * any, otherwise insert last.
      */
-    if (f->c->pending_filters) {
+    if (pendings) {
         for (next = f->next; next && !found; next = next->next) {
-            for (e = APR_RING_FIRST(f->c->pending_filters);
-                 e != APR_RING_SENTINEL(f->c->pending_filters,
-                                        ap_filter_t, pending);
+            for (e = APR_RING_FIRST(pendings);
+                 e != APR_RING_SENTINEL(pendings, ap_filter_t, pending);
                  e = APR_RING_NEXT(e, pending)) {
                 if (e == next) {
-                    found = e;
+                    APR_RING_INSERT_BEFORE(e, f, pending);
+                    found = 1;
                     break;
                 }
             }
         }
     }
     else {
-        f->c->pending_filters = apr_palloc(f->c->pool,
-                                           sizeof(*f->c->pending_filters));
-        APR_RING_INIT(f->c->pending_filters, ap_filter_t, pending);
+        pendings = apr_palloc(f->c->pool, sizeof(ap_filter_ring_t));
+        APR_RING_INIT(pendings, ap_filter_t, pending);
+        *ref = pendings;
     }
-    if (found) {
-        APR_RING_INSERT_BEFORE(found, f, pending);
-    }
-    else {
-        APR_RING_INSERT_TAIL(f->c->pending_filters, f, ap_filter_t, pending);
+    if (!found) {
+        APR_RING_INSERT_TAIL(pendings, f, ap_filter_t, pending);
     }
  
     return OK;
@@ -998,7 +1008,7 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
     apr_bucket_brigade *bb;
     ap_filter_t *f;
 
-    if (!c->pending_filters) {
+    if (!c->pending_output_filters) {
         return DECLINED;
     }
 
@@ -1009,11 +1019,11 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
      * to be relevant in the previous ones (e.g. ap_request_core_filter()
      * won't pass its buckets if its next filters yield already).
      */
-    for (f = APR_RING_LAST(c->pending_filters);
-         f != APR_RING_SENTINEL(c->pending_filters, ap_filter_t, pending);
+    for (f = APR_RING_LAST(c->pending_output_filters);
+         f != APR_RING_SENTINEL(c->pending_output_filters,
+                                ap_filter_t, pending);
          f = APR_RING_PREV(f, pending)) {
-        if (f->frec->direction == AP_FILTER_OUTPUT && f->bb
-                && !APR_BRIGADE_EMPTY(f->bb)) {
+        if (f->bb && !APR_BRIGADE_EMPTY(f->bb)) {
             apr_status_t rv;
 
             rv = ap_pass_brigade(f, bb);
@@ -1038,24 +1048,24 @@ AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c)
 {
     ap_filter_t *f;
 
-    if (!c->pending_filters) {
+    if (!c->pending_input_filters) {
         return DECLINED;
     }
 
-    for (f = APR_RING_LAST(c->pending_filters);
-         f != APR_RING_SENTINEL(c->pending_filters, ap_filter_t, pending);
+    for (f = APR_RING_LAST(c->pending_input_filters);
+         f != APR_RING_SENTINEL(c->pending_input_filters,
+                                ap_filter_t, pending);
          f = APR_RING_PREV(f, pending)) {
-        if (f->frec->direction == AP_FILTER_INPUT && f->bb) {
+        if (f->bb) {
             apr_bucket *e = APR_BRIGADE_FIRST(f->bb);
 
-            /* if there is at least one non-morphing bucket
+            /* if there is a leading non-morphing bucket
              * in place, then we have data pending
              */
             if (e != APR_BRIGADE_SENTINEL(f->bb)
                     && e->length != (apr_size_t)(-1)) {
                 return OK;
             }
-
         }
     }