httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tony Finch <...@dotat.at>
Subject Re: [PATCH] core_filter buffering
Date Thu, 28 Sep 2000 08:16:31 GMT

[I haven't looked at the patch yet because mutt won't display
content-type octet-stream attachments; I'll probably fix this...]

Bill Stoddard <stoddard@raleigh.ibm.com> wrote:
>
>The idea behind this patch (not fully implemented) is to save contents of bucket
>brigades in an array of iovecs hung off of core_filter->ctx. When the eos bucket
>arrives, the iovecs in core_filter->ctx are sent to the network using
>apr_sendv().  The patch also has an optimization to send the iovecs if the
>number of bytes represented by the iovecs exceeds some threshold value (8192
>bytes).  I can easily add other triggers to do network i/o (do not exceed x
>number of iovecs, etc.).

If you get a small bucket brigade (less than the write threshold)
it may be more efficient to copy all the data into one buffer than to
save it as an iovec; it'll certainly be more efficient to save the
bare bucket brigade than to make an iovec of it! Keep in mind the
length restriction on iovecs on SysV.

I'm not happy with the current brigade/iovec conversion API; I'm
inclined to think it is unnecessary and should be left to the core
filter.

>A few observations...
>First, I believe the chunked filter has a bug. The chunking headers are
>allocated off the stack. ap_create_bucket_transient() just saves away the
>pointers passed in to it, so the bucket is really only valid as long as the
>storage it points to. To fix this, I allocated the chunking headers out of the
>f->r->pool.  I can't claim to completely understand the filter philosophy, so
>I'm looking on comments on the appropriateness of this solution.

That's completely wrong. Response body data must NOT be allocated from
a pool bedause that can cause horrendous memory leaks. This situation
is exactly what the setaside() method is designed to handle. Are the
comments in ap_buckets.h not clear? Can I improve them?

>Second, auto_index generates huge numbers of small chunks. Seeing a response (in
>a single buffer) with 50 encoded 'chunks' is not esthetically pleasing  :-).  It
>seems that we need to introduce some sort of buffering heuristics (yea, with
>data copies) somewhere higher up (than core_filter) in the filter chain.
>Preferably in a place where it is transparently accessable to all modules.

There is a vague plan to make the ap_rput stuff more bucket-friendly,
which would help here. We also need to re-write mod_autoindex to use
buckets directly.

I'm going to try and look at all this more closely soon; I have a
patch in the pipeline to finalize the buckets API (allowing us to
optimise the storage layout without affecting other code) and once
I've committed that I'll be going over bucket-using code to convert it
and clean it up if necessary.

Tony.
-- 
en oeccget g mtcaa    f.a.n.finch
v spdlkishrhtewe y    dot@dotat.at
eatp o v eiti i d.    fanf@covalent.net

Mime
View raw message