* Transient bucket seems to be working fine in mod_sed.
* Added error handling code so that if ap_pass_brigade fails during
request processing, error is returned to sed_response_filter /
sed_request_filter.
Testing :
* Compiled with 2.2 branch and make sure there is no regression (against gsed
test cases).
* Compiled with trunk.
Final patch is attached.
Regards,
Basant.
On Mon, Sep 15, 2008 at 12:17:04AM -0700, Basant Kukreja wrote:
> Hi,
>
> Attached is the *rough* patch which uses transient buckets in mod_sed output
> filter.
>
> Testing :
> I created a 30MB and 300MB text files and ran OutputSed commands on the
> file.
> * Before the patch, process size (worker mpm with 1 thread) increased up to
> 300M
> for single request. After the patch, process size remains to 3MB to server
> 300M response output.
>
> I also removed 1 extra copying for processing output.
>
> I need to add some more error handling to finalize the patch. Any comments are
> welcome.
>
> Regards,
> Basant.
>
> On Thu, Sep 04, 2008 at 09:47:26PM -0500, William A. Rowe, Jr. wrote:
> > Basant Kukreja wrote:
> >>
> >> Based on your suggestion, I will check what are the other improvements from
> >> mod_substitute can be brought into mod_sed.
> >
> > Note that mod_substitute's brigade handling is already based on the work of
> > both Jim and Nick (author of mod_line_edit) - so they are pretty certain
> > that it is the right approach. Good idea to borrow from it.
> >
> > Bill
> Index: modules/filters/mod_sed.c
> ===
> --- modules/filters/mod_sed.c (revision 692768)
> +++ modules/filters/mod_sed.c (working copy)
> @@ -26,7 +26,8 @@
> #include "libsed.h"
>
> static const char *sed_filter_name = "Sed";
> -#define MODSED_OUTBUF_SIZE 4000
> +#define MODSED_OUTBUF_SIZE 8000
> +#define MAX_TRANSIENT_BUCKETS 50
>
> typedef struct sed_expr_config
> {
> @@ -44,11 +45,14 @@
> typedef struct sed_filter_ctxt
> {
> sed_eval_t eval;
> +ap_filter_t *f;
> request_rec *r;
> apr_bucket_brigade *bb;
> char *outbuf;
> char *curoutbuf;
> int bufsize;
> +apr_pool_t *tpool;
> +int numbuckets;
> } sed_filter_ctxt;
>
> module AP_MODULE_DECLARE_DATA sed_module;
> @@ -71,29 +75,68 @@
> sed_cfg->last_error = error;
> }
>
> +/* clear the temporary pool (used for transient buckets)
> + */
> +static void clear_ctxpool(sed_filter_ctxt* ctx)
> +{
> +apr_pool_clear(ctx->tpool);
> +ctx->outbuf = NULL;
> +ctx->curoutbuf = NULL;
> +ctx->numbuckets = 0;
> +}
> +
> +/* alloc_outbuf
> + * allocate output buffer
> + */
> +static void alloc_outbuf(sed_filter_ctxt* ctx)
> +{
> +ctx->outbuf = apr_palloc(ctx->tpool, ctx->bufsize + 1);
> +ctx->curoutbuf = ctx->outbuf;
> +}
> +
> +/* append_bucket
> + * Allocate a new bucket from buf and sz and append to ctx->bb
> + */
> +static void append_bucket(sed_filter_ctxt* ctx, char* buf, int sz)
> +{
> +int rv;
> +apr_bucket *b;
> +if (ctx->tpool == ctx->r->pool) {
> +/* We are not using transient bucket */
> +b = apr_bucket_pool_create(buf, sz, ctx->r->pool,
> + ctx->r->connection->bucket_alloc);
> +APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
> +}
> +else {
> +/* We are using transient bucket */
> +b = apr_bucket_transient_create(buf, sz,
> +ctx->r->connection->bucket_alloc);
> +APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
> +ctx->numbuckets++;
> +if (ctx->numbuckets >= MAX_TRANSIENT_BUCKETS) {
> +b = apr_bucket_flush_create(ctx->r->connection->bucket_alloc);
> +APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
> +rv = ap_pass_brigade(ctx->f->next, ctx->bb);
> +apr_brigade_cleanup(ctx->bb);
> +clear_ctxpool(ctx);
> +}
> +}
> +}
> +
> /*
> * flush_output_buffer
> * Flush the output data (stored in ctx->outbuf)
> */
> -static void flush_output_buffer(sed_filter_ctxt *ctx, char* buf, int sz)
> +static void flush_output_buffer(sed_filter_ctxt *ctx)
> {
> int size = ctx->curoutbuf - ctx->outbuf;
> char *out;
> -apr_bucket *b;
> -if (size + sz <= 0)
> +if ((ctx->outbuf == NULL) || (size <=0))
> return;
> -out = apr_palloc(ctx->r->pool, size + sz);
> -if (size) {
> -memcpy(out, ctx->outbuf, size);
> -}
> -if (buf && (sz > 0)) {
> -memcpy(out + size, buf, sz);
> -}
> -/* Reset the output buffer position */
> +out = apr_palloc(ctx->tpool, size);
> +memcpy(out, ctx->outbuf, size);
> +append_bucket(ctx, out, size);
> ctx->curoutbuf = ctx->outbuf;
> -b = apr_bucket_pool_create(out, size + sz, ctx->r->pool,
> - ctx->r->connection->bucket_alloc);
> -APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
> }
>
> /* This is a call back function. When libsed wants to gener
