Re: Memory consumption of mod_substitute

2007-12-07 Thread Justin Erenkrantz
On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
<[EMAIL PROTECTED]> wrote:
> * My test case lead to the exceptional situation of a very large passbb 
> bucket brigade
>   (about 1,000,000 buckets) as a result of processing 4 MB of the file. So I 
> add
>   a flush bucket once I have more than MAX_BUCKET (1000) buckets in the 
> brigade and pass it
>   down the chain to get it send and the passbb bucket brigade cleaned up and 
> its memory
>   reusable again.

Ha!  Is there a way we could be more aggressive in minimizing the
number of buckets mod_substitute creates?  Perhaps using
apr_bucket_copy to create ref-counted versions of the replacement
string?

> Comments, thoughts?

Your patch looks good on a quick cursory review.  -- justin


Re: Memory consumption of mod_substitute

2007-12-07 Thread Plüm , Rüdiger , VF-Group


> -Ursprüngliche Nachricht-
> Von: [EMAIL PROTECTED] 
> 
> Gesendet: Freitag, 7. Dezember 2007 11:24
> An: dev@httpd.apache.org
> Betreff: Re: Memory consumption of mod_substitute
> 
> 
> On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
> <[EMAIL PROTECTED]> wrote:
> > * My test case lead to the exceptional situation of a very 
> large passbb bucket brigade
> >   (about 1,000,000 buckets) as a result of processing 4 MB 
> of the file. So I add
> >   a flush bucket once I have more than MAX_BUCKET (1000) 
> buckets in the brigade and pass it
> >   down the chain to get it send and the passbb bucket 
> brigade cleaned up and its memory
> >   reusable again.
> 
> Ha!  Is there a way we could be more aggressive in minimizing the
> number of buckets mod_substitute creates?  Perhaps using
> apr_bucket_copy to create ref-counted versions of the replacement
> string?

Possibly in the strmatch case, but in the regexp case the replacement
string can be different each time when you use backreferences.
Yes, there maybe some optimization potential left here, but this might
become tricky with the temporary pools I introduced.

> 
> > Comments, thoughts?
> 
> Your patch looks good on a quick cursory review.  -- justin

Thanks for reviewing.

Regards

Rüdiger



Re: Memory consumption of mod_substitute

2007-12-08 Thread Jim Jagielski


On Dec 7, 2007, at 7:09 AM, Plüm, Rüdiger, VF-Group wrote:





-Ursprüngliche Nachricht-
Von: [EMAIL PROTECTED]

Gesendet: Freitag, 7. Dezember 2007 11:24
An: dev@httpd.apache.org
Betreff: Re: Memory consumption of mod_substitute


On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
<[EMAIL PROTECTED]> wrote:

* My test case lead to the exceptional situation of a very

large passbb bucket brigade

  (about 1,000,000 buckets) as a result of processing 4 MB

of the file. So I add

  a flush bucket once I have more than MAX_BUCKET (1000)

buckets in the brigade and pass it

  down the chain to get it send and the passbb bucket

brigade cleaned up and its memory

  reusable again.


Ha!  Is there a way we could be more aggressive in minimizing the
number of buckets mod_substitute creates?  Perhaps using
apr_bucket_copy to create ref-counted versions of the replacement
string?


Possibly in the strmatch case, but in the regexp case the replacement
string can be different each time when you use backreferences.
Yes, there maybe some optimization potential left here, but this might
become tricky with the temporary pools I introduced.


Yeppers... plus it makes the code itself much more
tricky with nasty paths :)

Re: Memory consumption of mod_substitute

2007-12-08 Thread Jim Jagielski




On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
<[EMAIL PROTECTED]> wrote:

* My test case lead to the exceptional situation of a very

large passbb bucket brigade

  (about 1,000,000 buckets) as a result of processing 4 MB

of the file. So I add

  a flush bucket once I have more than MAX_BUCKET (1000)

buckets in the brigade and pass it

  down the chain to get it send and the passbb bucket

brigade cleaned up and its memory

  reusable again.





By the by, even though we just use MAX_BUCKET here, it does
seem that there is high potential for naming conflicts
for this define... Maybe prefix it with some AP_* junk
just in case?

Re: Memory consumption of mod_substitute

2007-12-08 Thread Ruediger Pluem


On 12/08/2007 07:41 PM, Jim Jagielski wrote:
> 

 On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
 <[EMAIL PROTECTED]> wrote:
> * My test case lead to the exceptional situation of a very
 large passbb bucket brigade
>   (about 1,000,000 buckets) as a result of processing 4 MB
 of the file. So I add
>   a flush bucket once I have more than MAX_BUCKET (1000)
 buckets in the brigade and pass it
>   down the chain to get it send and the passbb bucket
 brigade cleaned up and its memory
>   reusable again.


> 
> By the by, even though we just use MAX_BUCKET here, it does
> seem that there is high potential for naming conflicts
> for this define... Maybe prefix it with some AP_* junk
> just in case?

Fixed in r602533. You may want to add it to the backport proposal as well.

Regards

Rüdiger



Re: Memory consumption of mod_substitute

2007-12-08 Thread Jim Jagielski


On Dec 8, 2007, at 2:31 PM, Ruediger Pluem wrote:




On 12/08/2007 07:41 PM, Jim Jagielski wrote:




On Dec 5, 2007 8:36 AM, Plüm, Rüdiger, VF-Group
<[EMAIL PROTECTED]> wrote:

* My test case lead to the exceptional situation of a very

large passbb bucket brigade

  (about 1,000,000 buckets) as a result of processing 4 MB

of the file. So I add

  a flush bucket once I have more than MAX_BUCKET (1000)

buckets in the brigade and pass it

  down the chain to get it send and the passbb bucket

brigade cleaned up and its memory

  reusable again.





By the by, even though we just use MAX_BUCKET here, it does
seem that there is high potential for naming conflicts
for this define... Maybe prefix it with some AP_* junk
just in case?


Fixed in r602533. You may want to add it to the backport proposal  
as well.




Already done... I updated the rev2 patch in place, since I figured
no one had looked at it yet :)