apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] brigade buffering.
Date Fri, 26 Jan 2001 10:25:12 GMT
As I've mentioned before, I share the same concerns as Greg Marr with this
particular approach (the transient buckets and large copies; although his
first example sounds more like a bug (which is okay) than an inherent design
issue).

Re: the followups. Again, I'm with Greg. We can't create a brigade API and
then say it is Apache-internal, or that people shouldn't use it. That
indicates a problem in the design/approach itself.

Cheers,
-g

On Thu, Jan 25, 2001 at 05:28:13PM -0500, Greg Marr wrote:
> 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"

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

Mime
View raw message