httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: Question about APR_BRIGADE_INSERT_TAIL usage
Date Wed, 27 Feb 2013 08:20:42 GMT
On 26 Feb 2013, at 7:52 AM, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:

> in the old PR51699, Stefan Fritsch turned a sequence like:
>     APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_pool_create(...));
> into:
>     apr_brigade_putstrs(bb, NULL,...);
> 
> (http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_include.c?r1=1159896&r2=1159895&pathrev=1159896)
> 
> 
> This kind of construction can be found in several places of httpd (and I was wondering
if the same kind of transformation could be a win.
> 
> 
> 
> My understanding is that:
>   - apr_brigade_[putc|puts|write...] try to reuse last bucket if possible, avoiding memory
allocation
>   - if needed (not enough space available, not allowed to write in the last bucket),
it creates a heap bucket of 8k or more (or transient one if flushed is requested))
>   - and then call APR_BRIGADE_INSERT_TAIL
> 
> So the 2 constructions are quite similar.
> 
> 
> Doing so would:
>   + try to use already allocated memory, so reduce memory footprint
>   + avoid memory fragmentation by avoiding small buckets
>   + reduce code length, so improve readability (IMO)
>   - BUT, would allocate, when needed, 8k of memory at a time instead of using the memory
already available in a pool, so increasing memory footprint...
> 
> 
> I haven't done any tests yet, so I don't have measurement on memory usage so far.
> Anyway, any thought on this approach would be appreciated.

Another set of functions that are very inefficient are the following below. As I recall they
existed to provide a smooth migration path for httpd v1.3 modules:

AP_DECLARE(int) ap_rputc(int c, request_rec *r);
AP_DECLARE(int) ap_rwrite(const void *buf, int nbyte, request_rec *r);
[etc]

In turn, they expand into this:

static apr_status_t buffer_output(request_rec *r,
                                  const char *str, apr_size_t len)
{
    conn_rec *c = r->connection;
    ap_filter_t *f;
    old_write_filter_ctx *ctx;

    if (len == 0)
        return APR_SUCCESS;

    f = insert_old_write_filter(r);
    ctx = f->ctx;

    /* if the first filter is not our buffering filter, then we have to
     * deliver the content through the normal filter chain
     */
    if (f != r->output_filters) {
        apr_status_t rv;
        apr_bucket *b = apr_bucket_transient_create(str, len, c->bucket_alloc);
        APR_BRIGADE_INSERT_TAIL(ctx->tmpbb, b);

        rv = ap_pass_brigade(r->output_filters, ctx->tmpbb);
        apr_brigade_cleanup(ctx->tmpbb);
        return rv;
    }

    if (ctx->bb == NULL) {
        ctx->bb = apr_brigade_create(r->pool, c->bucket_alloc);
    }

    return ap_fwrite(f->next, ctx->bb, str, len);
}

Regards,
Graham
--


Mime
View raw message