Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Message-Id: <5.0.2.1.2.20010125171619.00a8dec0@pop.charter.net>
Date: Thu, 25 Jan 2001 17:28:13 -0500
To: dev@apr.apache.org
From: Greg Marr <gregm@alum.wpi.edu>
Subject: Re: [PATCH] brigade buffering.
In-Reply-To: <Pine.LNX.4.21.0101251343210.18077-100000@koj>
References: <5.0.2.1.2.20010125155304.00a868d0@pop.charter.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed

At 04:51 PM 01/25/2001, rbb@covalent.net wrote:
> > FYI: this has the potential to fail miserably:
>
>I need to think about these, but I think you are correct.

I can provide a walk-through of the code if you want.  I decided it 
was too long to post to the list.

>I am not proposing that this is bullet-proof code, just that it is 
>an idea that should be considered.

I started out trying to figure out how you were successfully 
memcpy'ing a block of unknown size into a fixed size buffer, then 
stumbled into the check_ function.  Those popped up while I was 
reading through it.

> > A third failure is that check_brigade_flush creates a transient
> > bucket and sticks it in the brigade.  Here's where this can fail:
> >
> >      char buf[APR_BUCKET_BUFF_SIZE * 2 + 1];
> >
> >      apr_vsnprintf(buf, APR_BUCKET_BUFF_SIZE * 2 + 1, fmt, va);
> >
> >      return apr_brigade_puts(b, buf);
>
>This is not an issue, since it will never happen.  Remember, that I 
>said up front that these functions are not designed to be called by 
>programmers.  These are designed to be called by Apache's internal 
>functions.  In this case, the programmer has called either ap_f* or 
>ap_r*, which passes things down the brigade.

Well, they're in APR, and being used by Apache programmers, so I 
don't see how they can be "designed to be called by Apache's internal 
functions."

>However, you are correct that in general this is an issue, the 
>solution is to always copy the data, but that has other 
>drawbacks.  So, there are two answers to this that I see.  #1, tell 
>the programmer that (s)he is out of luck if (s)he doesn't understand 
>the code.  #2, actually copy all the data.
>
>I have no problem telling the programmer they are out of luck, this 
>is open source after all.  I dislike copying the data, because it 
>means we could chew a lot of memory very quickly.

So then the rule has to be that if call a apr_brigade_* function that 
can generate more than APR_BUCKET_BUFF_SIZE data in a single call, 
then that brigade has to be output before the buffer is reused or 
goes out of scope?
It's hard to see how that fits well into a general brigade API.

Would a better solution perhaps be that the apr_brigade_* functions 
will always copy the data, and leave it up to the ap_f* functions to 
provide the optimization of using the bucket API directly for large 
blocks of data?  That seems to be a much safer way of doing things.

One other optimization that I noticed would probably be useful would 
be to have apr_brigade_putstrs written at the same level as 
apr_brigade_write such that you save the memcpy of apr_brigade_write 
after the apr_vsnprintf.

-- 
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"