Re: [PATCH] Refactor and split out buffer stack

Hi Alex,

Sorry for the long delay in looking at your patch. I finally looked at  
it this weekend. I can't recommend accepting this patch, but I'd like  
to come up with a way for you (and others) to integrate it with  
subclassing, with a minimum of cut-and-paste.

The main problem with the patch is the performance hit, which you did  
candidly mention. In particular, the patch rolls back a key  
optimization made in 1.3 - that each string output from a component  
requires just a concatenation, rather than a method call. Sadly, we  
don't have a good benchmark suite in Mason to test the typical  
performance impact of changes (it's hard to know what a "typical" use  
of Mason is), and running the test suite may unfortunately be a poor  
substitute because the vast majority of test pages and components are  
pretty simple. But at Amazon, where many of the 1.3 optimizations were  
implemented, the concatenation optimzation did make a small-but- 
noticeable improvement in total page cost. I can't see hitting the  
vast majority of users who will never use this new feature with even a  
small performance penalty.

As far as the flush_buffer related bug fixes. I am inclined to handle  
those in a different way, but that's a large topic that I'll put in a  
separate email.

As far as integrating String::BufferStack with subclassing - if you  
set the enable_auto_flush property, Mason will call '$m->print'  
whenever anything is output in a component. You can then override  
print() to do what you want. Is this sufficient, or does it at least  
make things easier? If not, I'd like to work with you further on it.

Thanks
Jon

On Nov 6, 2008, at 7:38 PM, Alex Vandiver wrote:

> On Thu, 2008-11-06 at 21:13 -0600, Dave Rolsky wrote:
>> On Thu, 6 Nov 2008, John Williams wrote:
>>
>>> I, for one, would feel a lot better about it if  
>>> String::BufferStack were
>>> on CPAN.  Adding a dependancy on a non-cpan module is a definite  
>>> no vote
>>> from me, without even considering any other merits.
>>
>> I'm sure Alex would release this to CPAN if it were going to be  
>> part of
>> Mason, but for now it's just an experiment to see if he can unify the
>> Mason & TD buffers.
>
> Oh, absolutely -- it's on its way there now, in case that was causing
> people to hesitate looking at this.  I wasn't going to push it to CPAN
> unless people thought the experiment was worth looking at.
>
> To be clear, the experiment is a success, from my point of view.  The
> diffstat to Jifty removes a bunch of crufty code, and allows
> inter-calling between the templating systems that wasn't possible
> before.  For instance, you can $m->scomp a Template::Declare template,
> and it Just Works.  Hence why I'm interested in getting this patch
> applied.
>
> At this point, I want to know what the chances of inclusion into the
> mason core are.  If this patch were possible to do with subclassing  
> the
> request object instead, I'd do it and not trouble trunk with the  
> changes
> -- the difficulty is that the buffer code is more or less  
> everywhere, so
> there's no clean way to subclass the request object without copying  
> more
> or less the whole file.
> - Alex	
>
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's  
> challenge
> Build the coolest Linux based applications with Moblin SDK & win  
> great prizes
> Grand prize is a trip for two to an Open Source event anywhere in  
> the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Mason-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/mason-devel
>


------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/