httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Bloom <...@covalent.net>
Subject Re: [PATCH] resubmit new and improved ap_vrprintf fix to handle > 4K
Date Tue, 11 Sep 2001 06:26:43 GMT

Yeah, the OLD_WRITE filter removes a lot of the optimizations we should
be able to get with the filtering logic.  If you take a look at the apr_brigade_*
functions, you will see a more direct way to do this stuff.  We get direct access
to the buffers, and assuming the buffer was already allocated, it is easy to
just append to it.

IMHO, the OLD_WRITE filter was a mistake, but the group decided that it was
the right way to go.  I would be interested in seeing just how the performance
of the server changes by making ap_r* simple wrappers around apr_brigade_*.
The code was written to work as macros for most of the functions, so it should
be easy to implement.

Ryan

On Monday 10 September 2001 21:13, dean gaudet wrote:
> buhhh?
>
> you guys built a zero-copy infrastructure and you're putting in a one-copy
> sprintf?  i don't get it.  this new ap_rsprintf is less efficient than the
> ap_bsprintf in apache-1.3.
>
> also, wow, ap_rputc really sucks now.  that's a lot of code (in
> buffer_output) compared to:
>
>     *buf++ = c;
>
> guess all your abstraction has hidden the real buffer and you can't do any
> useful optimisations?
>
> this is one of the reasons i kept saying there's a reason there's a FILE *
> interface and a read(2)/write(2) interface :)
>
> oh ouch i gotta stop looking:
>
>     /* this will typically exit on the first test */
>     for (f = r->output_filters; f != NULL; f = f->next)
>         if (strcasecmp("OLD_WRITE", f->frec->name) == 0)
>             break;
>
> i'm puking.  strcasecmp???? strings??
>
> ok enough new-httpd for a while :)
>
> -dean
>
> On Wed, 15 Aug 2001, Cody Sherr wrote:
> > Small changes, AP_IOBUFSIZE macro (8K) for the buffer size, seemed
> > appropriate, and using local buffer.
> >
> > Thanks for the feedback.
> >
> > --
> > Cody Sherr
> >
> > Engineer
> > Covalent Technologies
> >
> > phone: (415)536-5292
> > email: csherr@covalent.net
> >
> >
> > Index: protocol.c
> > ===================================================================
> > RCS file: /home/cvspublic/httpd-2.0/server/protocol.c,v
> > retrieving revision 1.38
> > diff -u -r1.38 protocol.c
> > --- protocol.c	2001/08/07 16:40:25	1.38
> > +++ protocol.c	2001/08/15 23:04:07
> > @@ -1133,19 +1133,63 @@
> >      return nbyte;
> >  }
> >
> > +struct ap_vrprintf_data {
> > +    apr_vformatter_buff_t vbuff;
> > +    request_rec *r;
> > +    char *buff;
> > +};
> > +
> > +static apr_status_t r_flush(apr_vformatter_buff_t *buff)
> > +{
> > +    /* callback function passed to ap_vformatter to be called when
> > +     * vformatter needs to write into buff and buff.curpos > buff.endpos
> > */ +
> > +    /* ap_vrprintf_data passed as a apr_vformatter_buff_t, which is then
> > +     * "downcast" to an ap_vrprintf_data */
> > +    struct ap_vrprintf_data *vd = (struct ap_vrprintf_data*)buff;
> > +
> > +    if (vd->r->connection->aborted)
> > +        return -1;
> > +
> > +    /* r_flush is called when vbuff is completely full */
> > +    if (buffer_output(vd->r, vd->buff, AP_IOBUFSIZE)) {
> > +	return -1;
> > +    }
> > +
> > +    /* reset the buffer position */
> > +    vd->vbuff.curpos = vd->buff;
> > +    vd->vbuff.endpos = vd->buff + AP_IOBUFSIZE;
> > +
> > +    return APR_SUCCESS;
> > +}
> > +
> >  AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list va)
> >  {
> > -    char buf[4096];
> >      apr_size_t written;
> > +    struct ap_vrprintf_data vd;
> > +    static char vrprintf_buf[AP_IOBUFSIZE];
> > +
> > +    vd.vbuff.curpos = vrprintf_buf;
> > +    vd.vbuff.endpos = vrprintf_buf + AP_IOBUFSIZE;
> > +    vd.r = r;
> > +    vd.buff = vrprintf_buf;
> >
> >      if (r->connection->aborted)
> >          return -1;
> >
> > -    /* ### fix this mechanism to allow more than 4K of output */
> > -    written = apr_vsnprintf(buf, sizeof(buf), fmt, va);
> > +    written = apr_vformatter(r_flush, &vd.vbuff, fmt, va);
> > +    /* tack on null terminator on remaining string */
> > +    *(vd.vbuff.curpos) = '\0';
> >
> > -    if (buffer_output(r, buf, written) != APR_SUCCESS)
> > -        return -1;
> > +    if (written != -1) {
> > +	int n = vd.vbuff.curpos - vrprintf_buf;
> > +
> > +	/* last call to buffer_output, to finish clearing the buffer */
> > +	if (buffer_output(r, vrprintf_buf,n) != APR_SUCCESS)
> > +	    return -1;
> > +
> > +	written += n;
> > +    }
> >
> >      return written;
> >  }

-- 

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Mime
View raw message