You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/08/21 00:00:59 UTC

[mp2 rfc] APR::Bucket new methods

I've added the $b->destroy and $b->remove methods as suggested by Joe, so 
now we won't have the (temporary, request or connection long) memory 
leaks. I'm still trying to find the idiomatic bb processing loop, where 
only some buckets are modified. The idea is to make the code as short as 
possible, making it less error-prone and of course making it faster.

There is no point creating a new bucket brigade when there is one already, 
all we need to do is to provide an easy way to do buckets replacing. So at 
the moment a typical bb loop processing looks like:

   for (my $b = $bb->first; $b; $b = $bb->next($b)) {

       last if $b->is_eos;

       if ($b->read(my $data)) {
           my $nb = APR::Bucket->new(uc $data);
           $b->insert_before($nb);
           $b->delete;
           $b = $nb;
       }
   }

now I want to replace the kludgy 3 lines of bucket replace with a new 
wrapper $b->replace($new_b); so we will end up with:

   for (my $b = $bb->first; $b; $b = $bb->next($b)) {

       last if $b->is_eos;

       if ($b->read(my $data)) {
           my $nb = APR::Bucket->new(uc $data);
           $b->replace($nb);
       }
   }

Or may be we can also provide replace_data() to accept a string instead of 
a bucket, to make things even shorter and faster. So then it'll become:

   for (my $b = $bb->first; $b; $b = $bb->next($b)) {

       last if $b->is_eos;

       if ($b->read(my $data)) {
           $b->replace_data(APR::Bucket->new(uc $data));
       }
   }

This brings us to an API which is almost as simple as the streaming API:

     while ($filter->read(my $buffer, 1024)) {
         $filter->print(lc $buffer);
     }

but which gives us much more control of what happens inside the filter, 
should someone care about other bucket types (and they should), allow to 
fetch more than one bb in the same filter invocation, etc.

BTW, the streaming API is buggy, since it loses any meta buckets, but EOS. 
e.g. in the above example, if an upstream filter has sent a FLUSH bucket, 
it'll get lost, rendering a filter that uses the streaming API as in 
improperly behaved one.

Thoughts?


-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2 rfc] APR::Bucket new methods

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:

>>    for (my $b = $bb->first; $b; $b = $bb->next($b)) {
>>
>>        last if $b->is_eos;
>>
>>        if ($b->read(my $data)) {
>>            $b->replace_data(APR::Bucket->new(uc $data));
>>        }
>>    }
> 
> 
> You must have meant : $b->replace_data(uc $data) right ?

Yes, thank you :)

>> BTW, the streaming API is buggy, since it loses any meta buckets, but 
>> EOS. e.g. in the above example, if an upstream filter has sent a FLUSH 
>> bucket, it'll get lost, rendering a filter that uses the streaming API 
>> as in improperly behaved one.
> 
> 
> Any particular reason it's losing those buckets that we can't address/fix ?

Not in the way the API is designed. The streaming API was designed to get 
the data out, think of it as $bb->flatten but a bit more effective, as it 
gets in chunks. Only later the check for eos was added. So there is no way 
to tell whether there was a flush bucket for example. And even if we could 
somehow handle that, what about other meta buckets? I think the streaming 
API shouldn't be used in things like CPAN modules, but is good for quick 
prototyping or in-house projects where you know exactly what other filters 
are being used and whether or not you care about losing those other 
buckets or not.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2 rfc] APR::Bucket new methods

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.

Stas Bekman wrote:
> I've added the $b->destroy and $b->remove methods as suggested by Joe, so 
> now we won't have the (temporary, request or connection long) memory 
> leaks. I'm still trying to find the idiomatic bb processing loop, where 
> only some buckets are modified. The idea is to make the code as short as 
> possible, making it less error-prone and of course making it faster.
> 
> There is no point creating a new bucket brigade when there is one already, 
> all we need to do is to provide an easy way to do buckets replacing. So at 
> the moment a typical bb loop processing looks like:
> 
>    for (my $b = $bb->first; $b; $b = $bb->next($b)) {
> 
>        last if $b->is_eos;
> 
>        if ($b->read(my $data)) {
>            my $nb = APR::Bucket->new(uc $data);
>            $b->insert_before($nb);
>            $b->delete;
>            $b = $nb;
>        }
>    }
> 
> now I want to replace the kludgy 3 lines of bucket replace with a new 
> wrapper $b->replace($new_b); so we will end up with:
> 
>    for (my $b = $bb->first; $b; $b = $bb->next($b)) {
> 
>        last if $b->is_eos;
> 
>        if ($b->read(my $data)) {
>            my $nb = APR::Bucket->new(uc $data);
>            $b->replace($nb);
>        }
>    }

This sure is cleaner and less error prone IMO, I like it.

> Or may be we can also provide replace_data() to accept a string instead of 
> a bucket, to make things even shorter and faster. So then it'll become:
> 
>    for (my $b = $bb->first; $b; $b = $bb->next($b)) {
> 
>        last if $b->is_eos;
> 
>        if ($b->read(my $data)) {
>            $b->replace_data(APR::Bucket->new(uc $data));
>        }
>    }

You must have meant : $b->replace_data(uc $data) right ?

In this case, I'd just find a better name, maybe. bucket->replace_data
doesn't really replace the data in the bucket, doesn't it? The name just
sounds counter-intuitive to me.

> This brings us to an API which is almost as simple as the streaming API:
> 
>      while ($filter->read(my $buffer, 1024)) {
>          $filter->print(lc $buffer);
>      }
> 
> but which gives us much more control of what happens inside the filter, 
> should someone care about other bucket types (and they should), allow to 
> fetch more than one bb in the same filter invocation, etc.
> 
> BTW, the streaming API is buggy, since it loses any meta buckets, but EOS. 
> e.g. in the above example, if an upstream filter has sent a FLUSH bucket, 
> it'll get lost, rendering a filter that uses the streaming API as in 
> improperly behaved one.

Any particular reason it's losing those buckets that we can't address/fix ?

> Thoughts?
> 
> 

-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

Re: [mp2 rfc] APR::Bucket new methods

Posted by Stas Bekman <st...@stason.org>.
I also think that the methods should return the object itself, so they can 
be stacked. e.g.:

   $b->remove->destroy;

one more missing method that is we must add is $b->setaside, so one can 
stash buckets away. e.g. if the stashing function is stash_away:

   stash_away($b->setaside); # $b->setaside also returns $b

for maximum performance we should return $self in the non-void context of 
course.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org