httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: ap_r* performance patch
Date Tue, 23 Jan 2001 05:14:35 GMT

> 1) ordering
> 
> 2) large writes in ap_rputs, ap_rvputs, ap_rwrite, and ap_vrprintf
>    specifically: how will you avoid copying the content into the brigade?

Please cross #2 off this list.  The following patch modifies
default_handler to print a string of 100,000 'a's using the ap_write
API.  This is done with NO copies, and no heap corruption.  The
ap_vrprintf can't be done without copying content easily, but the concept
is there.  BTW, this patch fixes all of them, except for ap_vrprintf,
mainly because I didn't want to spend much time on this today.  Sorry it
took so long.  I had this patch done over an hour ago, but I could't get a
diff.  :-(

Ryan


Index: include/httpd.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/httpd.h,v
retrieving revision 1.131
diff -u -d -b -w -u -r1.131 httpd.h
--- include/httpd.h	2001/01/20 06:05:14	1.131
+++ include/httpd.h	2001/01/23 05:03:27
@@ -85,6 +85,7 @@
 #include "apr_pools.h"
 #include "apr_time.h"
 #include "apr_network_io.h"
+#include "apr_buckets.h"
 
 #ifdef CORE_PRIVATE
 
@@ -809,6 +810,8 @@
     /** A flag to determine if the eos bucket has been sent yet
      *  @defvar int eos_sent */
     int eos_sent;
+
+    apr_bucket_brigade *bb;
 
 /* Things placed at the end of the record to avoid breaking binary
  * compatibility.  It would be nice to remember to reorder the entire
Index: modules/http/http_core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
retrieving revision 1.240
diff -u -d -b -w -u -r1.240 http_core.c
--- modules/http/http_core.c	2001/01/21 22:14:15	1.240
+++ modules/http/http_core.c	2001/01/23 05:03:29
@@ -2948,6 +2948,9 @@
     int errstatus;
     apr_file_t *fd = NULL;
     apr_status_t status;
+    char foobar[100000];
+    int i;
+
     /* XXX if/when somebody writes a content-md5 filter we either need to
      *     remove this support or coordinate when to use the filter vs.
      *     when to use this code
@@ -3021,6 +3024,7 @@
                        ap_md5digest(r->pool, fd));
     }
 
+#if 0
     bb = apr_brigade_create(r->pool);
     e = apr_bucket_create_file(fd, 0, r->finfo.size);
 
@@ -3029,6 +3033,13 @@
     APR_BRIGADE_INSERT_TAIL(bb, e);
 
     return ap_pass_brigade(r->output_filters, bb);
+#endif
+
+    for (i=0;i<100000;i++) {
+        foobar[i] = 'a';
+    }
+    ap_rwrite(foobar, 100000, r);
+    return 0;
 }
 
 /*
Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.269
diff -u -d -b -w -u -r1.269 http_protocol.c
--- modules/http/http_protocol.c	2001/01/22 21:57:57	1.269
+++ modules/http/http_protocol.c	2001/01/23 05:03:32
@@ -1619,13 +1619,17 @@
 
 static void end_output_stream(request_rec *r)
 {
-    apr_bucket_brigade *bb;
     apr_bucket *b;
 
-    bb = apr_brigade_create(r->pool);
+    if (r->bb) {
+        apr_brigade_flush(r->bb);
+    }
+    else {
+        r->bb = apr_brigade_create(r->pool);
+    }
     b = apr_bucket_create_eos();
-    APR_BRIGADE_INSERT_TAIL(bb, b);
-    ap_pass_brigade(r->output_filters, bb);
+    APR_BRIGADE_INSERT_TAIL(r->bb, b);
+    ap_pass_brigade(r->output_filters, r->bb);
 }
 
 void ap_finalize_sub_req_protocol(request_rec *sub)
@@ -2998,77 +3002,130 @@
 
 AP_DECLARE(int) ap_rputc(int c, request_rec *r)
 {
-    apr_bucket_brigade *bb = NULL;
     apr_bucket *b;
+    apr_status_t status;
     char c2 = (char)c;
 
     if (r->connection->aborted) {
 	return EOF;
     }
 
-    bb = apr_brigade_create(r->pool);
-    b = apr_bucket_create_transient(&c2, 1);
-    APR_BRIGADE_INSERT_TAIL(bb, b);
-    ap_pass_brigade(r->output_filters, bb);
+    if (!r->bb) {
+        r->bb = apr_brigade_create(r->pool);
+    }
 
-    return c;
+    b = APR_BRIGADE_LAST(r->bb);
+
+    status = apr_brigade_putc(r->bb, c2);
+
+    /* Okay, so this sucks, but we don't have many options.  What we
+     * are doing here is just checking to see if we just converted the
+     * buffer into a new bucket and put it at the end of the brigade.  If
+     * we did, we want to pass the brigade to the next filter.  If not,
+     * we just keep going.  This allows us to use the network to limit how
+     * much data we send at any one time.
+     */
+    if (b != APR_BRIGADE_LAST(r->bb)) {
+        ap_pass_brigade(r->output_filters, r->bb);
+    }
+
+    return status;
 }
 
 AP_DECLARE(int) ap_rputs(const char *str, request_rec *r)
 {
-    apr_bucket_brigade *bb = NULL;
     apr_bucket *b;
-    apr_size_t len;
+    apr_status_t status;
 
     if (r->connection->aborted)
         return EOF;
     if (*str == '\0')
         return 0;
 
-    len = strlen(str);
-    bb = apr_brigade_create(r->pool);
-    b = apr_bucket_create_transient(str, len);
-    APR_BRIGADE_INSERT_TAIL(bb, b);
-    ap_pass_brigade(r->output_filters, bb);
+    if (!r->bb) {
+        r->bb = apr_brigade_create(r->pool);
+    }
 
-    return len;
+    b = APR_BRIGADE_LAST(r->bb);
+
+    status = apr_brigade_puts(r->bb, str);
+
+    /* Okay, so this sucks, but we don't have many options.  What we
+     * are doing here is just checking to see if we just converted the
+     * buffer into a new bucket and put it at the end of the brigade.  If
+     * we did, we want to pass the brigade to the next filter.  If not,
+     * we just keep going.  This allows us to use the network to limit how
+     * much data we send at any one time.
+     */
+    if (b != APR_BRIGADE_LAST(r->bb)) {
+        ap_pass_brigade(r->output_filters, r->bb);
 }
 
+    return status;
+}
+
 AP_DECLARE(int) ap_rwrite(const void *buf, int nbyte, request_rec *r)
 {
-    apr_bucket_brigade *bb = NULL;
     apr_bucket *b;
+    apr_status_t status;
 
     if (r->connection->aborted)
         return EOF;
     if (nbyte == 0)
         return 0;
 
-    bb = apr_brigade_create(r->pool);
-    b = apr_bucket_create_transient(buf, nbyte);
-    APR_BRIGADE_INSERT_TAIL(bb, b);
-    ap_pass_brigade(r->output_filters, bb);
-    return nbyte;
+    if (!r->bb) {
+        r->bb = apr_brigade_create(r->pool);
+    }
+
+    b = APR_BRIGADE_LAST(r->bb);
+
+    status = apr_brigade_write(r->bb, buf, nbyte);
+
+    /* Okay, so this sucks, but we don't have many options.  What we
+     * are doing here is just checking to see if we just converted the
+     * buffer into a new bucket and put it at the end of the brigade.  If
+     * we did, we want to pass the brigade to the next filter.  If not,
+     * we just keep going.  This allows us to use the network to limit how
+     * much data we send at any one time.
+     */
+    if (b != APR_BRIGADE_LAST(r->bb)) {
+        ap_pass_brigade(r->output_filters, r->bb);
 }
 
+    return status;
+}
+
 AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list va)
 {
-    apr_bucket_brigade *bb = NULL;
-    apr_size_t written;
+    apr_bucket *b;
+    apr_status_t status;
 
     if (r->connection->aborted)
         return EOF;
 
-    bb = apr_brigade_create(r->pool);
-    written = apr_brigade_vprintf(bb, fmt, va);
-    if (written != 0)
-        ap_pass_brigade(r->output_filters, bb);
-    return written;
+    if (!r->bb) {
+        r->bb = apr_brigade_create(r->pool);
 }
 
-/* TODO:  Make ap pa_bucket_vprintf that printfs directly into a
- * bucket.
+    b = APR_BRIGADE_LAST(r->bb);
+
+    status = apr_brigade_vprintf(r->bb, fmt, va);
+
+    /* Okay, so this sucks, but we don't have many options.  What we
+     * are doing here is just checking to see if we just converted the
+     * buffer into a new bucket and put it at the end of the brigade.  If
+     * we did, we want to pass the brigade to the next filter.  If not,
+     * we just keep going.  This allows us to use the network to limit how
+     * much data we send at any one time.
  */
+    if (b != APR_BRIGADE_LAST(r->bb)) {
+        ap_pass_brigade(r->output_filters, r->bb);
+    }
+        
+    return status;
+}
+
 AP_DECLARE_NONSTD(int) ap_rprintf(request_rec *r, const char *fmt, ...)
 {
     va_list va;
@@ -3086,31 +3143,34 @@
 
 AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r, ...)
 {
-    apr_bucket_brigade *bb = NULL;
     apr_size_t written;
     va_list va;
 
     if (r->connection->aborted)
         return EOF;
-    bb = apr_brigade_create(r->pool);
+
+    if (!r->bb) {
+        r->bb = apr_brigade_create(r->pool);
+    }
+
     va_start(va, r);
-    written = apr_brigade_vputstrs(bb, va);
+    written = apr_brigade_vputstrs(r->bb, va);
     va_end(va);
-    if (written != 0)
-        ap_pass_brigade(r->output_filters, bb);
     return written;
 }
 
 AP_DECLARE(int) ap_rflush(request_rec *r)
 {
     /* we should be using a flush bucket to flush the stack, not buff code. */
-    apr_bucket_brigade *bb;
     apr_bucket *b;
 
-    bb = apr_brigade_create(r->pool);
     b = apr_bucket_create_flush();
-    APR_BRIGADE_INSERT_TAIL(bb, b);
-    ap_pass_brigade(r->output_filters, bb);
+    if (!r->bb) {
+        r->bb = apr_brigade_create(r->pool);
+    }
+    apr_brigade_flush(r->bb);
+    APR_BRIGADE_INSERT_TAIL(r->bb, b);
+    ap_pass_brigade(r->output_filters, r->bb);
     return 0;
 }
 
Index: server/util_filter.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/util_filter.c,v
retrieving revision 1.43
diff -u -d -b -w -u -r1.43 util_filter.c
--- server/util_filter.c	2001/01/19 07:04:29	1.43
+++ server/util_filter.c	2001/01/23 05:03:33
@@ -237,6 +237,7 @@
              */
             next->r->eos_sent = 1;
         }
+        apr_brigade_flush(bb);
         return next->frec->filter_func.out_func(next, bb);
     }
     return AP_NOBODY_WROTE;
Index: srclib/apr-util/buckets/apr_brigade.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
retrieving revision 1.3
diff -u -d -b -w -u -r1.3 apr_brigade.c
--- srclib/apr-util/buckets/apr_brigade.c	2001/01/19 07:01:59	1.3
+++ srclib/apr-util/buckets/apr_brigade.c	2001/01/23 05:03:46
@@ -99,6 +99,9 @@
 
     b = apr_palloc(p, sizeof(*b));
     b->p = p;
+
+    b->start = b->end = NULL;
+
     APR_RING_INIT(&b->list, apr_bucket, link);
 
     apr_register_cleanup(b->p, b, apr_brigade_cleanup, apr_brigade_cleanup);
@@ -185,12 +188,41 @@
     return vec - orig;
 }
 
+static int check_brigade_flush(char *start, char **end, const char *str, 
+                               apr_size_t *n, apr_bucket_brigade *b)
+{
+    if (*n > (APR_BUCKET_BUFF_SIZE - (*end - start))) {
+        if (start) {
+            memcpy(*end, str, APR_BUCKET_BUFF_SIZE - (*end - start));
+            *n -= (APR_BUCKET_BUFF_SIZE - (*end - start));
+            *end += (APR_BUCKET_BUFF_SIZE - (*end - start));
+            apr_brigade_flush(b);
+        }
+        if (*n > APR_BUCKET_BUFF_SIZE) {
+            apr_bucket *e = apr_bucket_create_transient(str, *n);
+            APR_BRIGADE_INSERT_TAIL(b, e);
+            return 1;
+        }
+    }
+    return 0;
+}
+
+APU_DECLARE(void) apr_brigade_flush(apr_bucket_brigade *b)
+{
+    apr_bucket *e;
+    apr_size_t i = b->end - b->start;
+
+    if (i) {
+        e = apr_bucket_create_heap(b->start, i, 0, NULL);
+        APR_BRIGADE_INSERT_TAIL(b, e);
+        b->start = b->end = NULL;
+    }
+}
+
 APU_DECLARE(int) apr_brigade_vputstrs(apr_bucket_brigade *b, va_list va)
 {
-    apr_bucket *r;
     const char *x;
     int j, k;
-    apr_size_t i;
 
     for (k = 0;;) {
         x = va_arg(va, const char *);
@@ -198,20 +230,52 @@
             break;
         j = strlen(x);
        
-	/* XXX: copy or not? let the caller decide? */
-        r = apr_bucket_create_heap(x, j, 1, &i);
-        if (i != j) {
-            /* Do we need better error reporting?  */
-            return -1;
+        k += apr_brigade_write(b, x, j);
         }
-        k += i;
+    return k;
+}
 
-        APR_BRIGADE_INSERT_TAIL(b, r);
+APU_DECLARE(int) apr_brigade_putc(apr_bucket_brigade *b, const char c)
+{
+    apr_size_t nbyte = 1;
+
+    if (check_brigade_flush(b->start, &b->end, &c, &nbyte, b)) { 
+        return 1;
     }
 
-    return k;
+    if (!b->start) {
+        b->start = b->end = malloc(APR_BUCKET_BUFF_SIZE);
+    }
+
+    memcpy(b->end, &c, 1);
+    b->end++;
+
+    return 1;
+}
+
+APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte)
+{
+    apr_size_t n = nbyte;
+
+    if (check_brigade_flush(b->start, &b->end, str, &nbyte, b)) { 
+        return n;
+    }
+
+    if (!b->start) {
+        b->start = b->end = malloc(APR_BUCKET_BUFF_SIZE);
+    }
+
+    memcpy(b->end, str, nbyte);
+    b->end += nbyte;
+
+    return n;
 }
 
+APU_DECLARE(int) apr_brigade_puts(apr_bucket_brigade *b, const char *str)
+{
+    return apr_brigade_write(b, str, strlen(str));
+}
+
 APU_DECLARE_NONSTD(int) apr_brigade_putstrs(apr_bucket_brigade *b, ...)
 {
     va_list va;
@@ -240,13 +304,8 @@
      * directly into a bucket.  I'm being lazy right now.  RBB
      */
     char buf[4096];
-    apr_bucket *r;
-    int res;
-
-    res = apr_vsnprintf(buf, 4096, fmt, va);
 
-    r = apr_bucket_create_heap(buf, strlen(buf), 1, NULL);
-    APR_BRIGADE_INSERT_TAIL(b, r);
+    apr_vsnprintf(buf, 4096, fmt, va);
 
-    return res;
+    return apr_brigade_puts(b, buf);
 }
Index: srclib/apr-util/include/apr_buckets.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_buckets.h,v
retrieving revision 1.65
diff -u -d -b -w -u -r1.65 apr_buckets.h
--- srclib/apr-util/include/apr_buckets.h	2001/01/22 23:11:32	1.65
+++ srclib/apr-util/include/apr_buckets.h	2001/01/23 05:03:47
@@ -78,6 +78,8 @@
  * @package Bucket Brigades
  */
 
+#define APR_BUCKET_BUFF_SIZE 9000
+
 typedef enum {APR_BLOCK_READ, APR_NONBLOCK_READ} apr_read_type_e;
 
 /*
@@ -237,6 +239,8 @@
      *  the destroying function is responsible for killing the cleanup.
      */
     apr_pool_t *p;
+    char *start;
+    char *end;
     /** The buckets in the brigade are on this list. */
     /*
      * XXX: the apr_bucket_list structure doesn't actually need a name tag
@@ -628,9 +632,10 @@
 APU_DECLARE(int) apr_brigade_to_iovec(apr_bucket_brigade *b, 
 				     struct iovec *vec, int nvec);
 
+APU_DECLARE(void) apr_brigade_flush(apr_bucket_brigade *b);
+
 /**
- * This function writes a list of strings into a bucket brigade.  We just 
- * allocate a new heap bucket for each string.
+ * This function writes a list of strings into a bucket brigade. 
  * @param b The bucket brigade to add to
  * @param va A list of strings to add
  * @return The number of bytes added to the brigade
@@ -639,8 +644,34 @@
 APU_DECLARE(int) apr_brigade_vputstrs(apr_bucket_brigade *b, va_list va);
 
 /**
+ * This function writes an string into a bucket brigade.
+ * @param b The bucket brigade to add to
+ * @param str The string to add
+ * @return The number of bytes added to the brigade
+ * @deffunc int apr_brigade_write(ap_bucket_brigade *b, const char *str)
+ */
+APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte);
+
+/**
+ * This function writes an string into a bucket brigade.
+ * @param b The bucket brigade to add to
+ * @param str The string to add
+ * @return The number of bytes added to the brigade
+ * @deffunc int apr_brigade_puts(ap_bucket_brigade *b, const char *str)
+ */
+APU_DECLARE(int) apr_brigade_puts(apr_bucket_brigade *b, const char *str);
+
+/**
+ * This function writes a character into a bucket brigade.
+ * @param b The bucket brigade to add to
+ * @param c The character to add
+ * @return The number of bytes added to the brigade
+ * @deffunc int apr_brigade_putc(apr_bucket_brigade *b, const char c)
+ */
+APU_DECLARE(int) apr_brigade_putc(apr_bucket_brigade *b, const char c);
+
+/**
  * This function writes an unspecified number of strings into a bucket brigade.
- * We just allocate a new heap bucket for each string.
  * @param b The bucket brigade to add to
  * @param ... The strings to add
  * @return The number of bytes added to the brigade
@@ -649,7 +680,7 @@
 APU_DECLARE_NONSTD(int) apr_brigade_putstrs(apr_bucket_brigade *b, ...);
 
 /**
- * Evaluate a printf and put the resulting string into a bucket at the end 
+ * Evaluate a printf and put the resulting string at the end 
  * of the bucket brigade.
  * @param b The brigade to write to
  * @param fmt The format of the string to write
@@ -661,7 +692,7 @@
                                           const char *fmt, ...);
 
 /**
- * Evaluate a printf and put the resulting string into a bucket at the end 
+ * Evaluate a printf and put the resulting string at the end 
  * of the bucket brigade.
  * @param b The brigade to write to
  * @param fmt The format of the string to write


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------





Mime
View raw message