> 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
-------------------------------------------------------------------------------
|