Re: [perl #76674] [PATCH] Locale::Maketext - speed and efficiency tweaks

On Sep 26, 2010, at 7:28 PM, Father Chrysostomos via RT wrote:

> On Thu Jul 22 11:09:33 2010, toddr@cpanel.net wrote:
>> This patch resolves CPAN RT #48118 of the same name.
>> 
>> The change checks for [ in the string before it attempts to compile
>>   it. If [ is not found, it is able to bypass compile. We've found
>>   that this change benchmarked 9% faster in our code.
> 
> In my experience, $a !~ /\[/ is faster than $a !~ y/[//, because the
> former can stop when it finds a [ and doesn’t have to keep count the way
> y/// does.
> 
> Using Steffen Mueller’s dumbbench script, I confirmed that /\[/ takes
> longer to compile, but y/[// takes longer to run.
> 
> Also, from reading the source (not actually testing this) it seems that
> your patch stops an unmatched clasing bracket from being an error. Am I
> right about this?
> 

Thanks for your careful review of this patch. You were right that the patch short circuits error conditions. I've simplified the patch. I'm pasting the commit message below since it's got all of my thoughts in it:

    RT 76668 - Check string to compile for chars ~][ and return \"$string" if not found.
    This is a 250% speed improvement on strings which don't require compile and only a
    ~2% hit if they did need compiling.
    
    Remove \G since everything is being captured it has no value. This means we don't
    have to worry about seting pos $string_to_compile = 0 to prevent the previous
    regex from affecting this one. There is a negligible speed improvement removing
    the \G

All tests continue to pass.

FYI if you want to benchmark on your own, this is the method I used to show the speedup:

Testing compiled string performance:
perl -Ilib -MBenchmark -MLocale::Maketext -le 'package X;our @ISA=qw(Locale::Maketext);package X::en;our @ISA=qw(X);our %Lexicon = (_AUTO => 1);package main;my $h=X->get_handle(); my $c = 1; timethis(100000, sub{$h->maketext("bar $c [_1]", $c); $c++})'

Testing strings which don't need compile
perl -Ilib -MBenchmark -MLocale::Maketext -le 'package X;our @ISA=qw(Locale::Maketext);package X::en;our @ISA=qw(X);our %Lexicon = (_AUTO => 1);package main;my $h=X->get_handle(); my $c = 1; timethis(800000, sub{$h->maketext("bar $c"); $c++})'

• [perl #76674] [PATCH] Locale::Maketext - speed and efficiency tweaks by Father Chrysostomos via RT
• Re: [perl #76674] [PATCH] Locale::Maketext - speed and efficiency tweaks by Todd Rinaldo
• [perl #76674] [PATCH] Locale::Maketext - speed and efficiency tweaks by Father Chrysostomos via RT