Re: cvs commit: apr-util/buckets apr_brigade.c

[Greg Stein <gs...@lyra.org>]

On Sun, Sep 29, 2002 at 01:05:38AM -0000, brianp@apache.org wrote:
> brianp      2002/09/28 18:05:38
> 
>   Modified:    buckets  apr_brigade.c
>   Log:
>   Rewrite of apr_brigade_writev().  It's now more efficient for
>   both large and small inputs: zero-copy for data larger than 8KB,
>   fewer operations (and fewer branches) per for-loop iteration
>   for the <= 8KB case.

Cool!

I presume you found the "count the length up front" to have no impact in the
short case? Thus, going ahead and doing the count and then the switch to
transient buckets?

One thing I just realized, though: if there is no flush function, then a
transient could be a problem. I just double-checked apr_brigade_write() and
it switches over to HEAP buckets for that case. Should _writev follow the
same pattern?

btw, good call on the flush in between the two HEAP buckets. I woulda missed
that, and it could be a Bad Thing [if a _writev added a little, added a
little, added a little... and now you've got a billion HEAP buckets].

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr-util/buckets apr_brigade.c

[Greg Stein <gs...@lyra.org>]

On Sun, Sep 29, 2002 at 06:55:27PM -0700, Brian Pane wrote:
> On Sun, 2002-09-29 at 17:19, Greg Stein wrote:
>...
> > One thing I just realized, though: if there is no flush function, then a
> > transient could be a problem. I just double-checked apr_brigade_write() and
> > it switches over to HEAP buckets for that case. Should _writev follow the
> > same pattern?
> 
> You mean, in case the caller then deletes the underlying storage for the
> strings in the iovec?  Yes, in that case we should use heap buckets.

Right.

> I'll make a change for this.

You da man!

> Both the _writev and the _write functions
> really need a warning in the documentation: "not guaranteed to be zero
> copy, so build your own buckets if you're trying to pass very large
> amounts of data."

Well... amend that a bit: the caller doesn't have to build buckets IF they
pass a flush function. It will be zero-copy when a flush func is provided.

Really... *all* calls should have a flush function to encourage zero copy
operation. If a flush function is not used, then the caller should revisit
what they're doing with the brigade.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr-util/buckets apr_brigade.c

[Brian Pane <br...@cnet.com>]

On Sun, 2002-09-29 at 17:19, Greg Stein wrote:
> On Sun, Sep 29, 2002 at 01:05:38AM -0000, brianp@apache.org wrote:
> > brianp      2002/09/28 18:05:38
> > 
> >   Modified:    buckets  apr_brigade.c
> >   Log:
> >   Rewrite of apr_brigade_writev().  It's now more efficient for
> >   both large and small inputs: zero-copy for data larger than 8KB,
> >   fewer operations (and fewer branches) per for-loop iteration
> >   for the <= 8KB case.
> 
> Cool!
> 
> I presume you found the "count the length up front" to have no impact in the
> short case? Thus, going ahead and doing the count and then the switch to
> transient buckets?

Right, counting the length up front added some overhead, but it also
eliminated the need for a lot of conditional and arithmetic operations
per iteration of the main copy loop, so the change basically paid for
itself.

> One thing I just realized, though: if there is no flush function, then a
> transient could be a problem. I just double-checked apr_brigade_write() and
> it switches over to HEAP buckets for that case. Should _writev follow the
> same pattern?

You mean, in case the caller then deletes the underlying storage for the
strings in the iovec?  Yes, in that case we should use heap buckets.
I'll make a change for this.  Both the _writev and the _write functions
really need a warning in the documentation: "not guaranteed to be zero
copy, so build your own buckets if you're trying to pass very large
amounts of data."

Brian