How can I improve this commify procedure for speed?

Question

I need an effective filter or routine for use with Template :: Toolkit. It should be used many times on the page. It must support decimal places.

This one is found in The Perl Cookbook :

sub commify {
    my $text = reverse $_[0];
    $text =~ s/(\d\d\d)(?=\d)(?!\d*\.)/$1,/g;
    return scalar reverse $text;
}

Are there more efficient ways?

Answer 1

Before you try to optimize something, make sure that this is really a problem. Use the profiler to find problem areas in your code and focus on those areas.

, , , , :

• - Memoize , .
• , .
• ,
• - -, .

Answer 2

, - .

commify 130000 , : "31243245356.4432".

, 10000 , 76 . , , 2-3 .

Answer 3

:

sub commify {
    my $text = shift;
    1 while $text =~ s/ ( \d ) ( \d{3} ) (\D|\z) /$1,$2$3/xms;
    return $text;
}

, , Benchmark .

#!/usr/bin/perl

use strict;
use warnings;
use Benchmark;

sub your_commify {
    my $text = reverse 100000000;
    $text =~ s/(\d\d\d)(?=\d)(?!\d*\.)/$1,/g;
    return scalar reverse $text;
}

sub my_commify {
    my $text = 100000000;
    1 while $text =~ s/ ( \d ) ( \d{3} ) (\D|\z) /$1,$2$3/xms;
    return $text;
}

timethese(
    -10,
    {
        'yours' => \&your_commify,
        'mine'  => \&my_commify,
    }
);

:

~$ ./benchmark.pl
Benchmark: running mine, yours for at least 10 CPU seconds...
mine: 10 wallclock secs (10.01 usr +  0.01 sys = 10.02 CPU) @ 111456.89/s (n=1116798)
yours: 11 wallclock secs (10.04 usr +  0.00 sys = 10.04 CPU) @ 250092.33/s (n=2510927)

, ~ 2,25 ! ( " 10 CPU" "n".)

, , ... Benchmark!

Answer 4

sub commify {
    my $n = $_[0];

    my $s = abs($n) != $n;
    my $x = index($n, '.');
    $x = length($n) if $x == -1;
    substr($n, $x, 0, ',') while ($x -= 3) > $s;
    return $n;
}

Answer 5

I agree with Brian and depesz: from a practical point of view, this function probably cannot start if you are trying to improve the performance of your application. However, you can write a much faster function commify. One way is to avoid regular expressions.

use strict;
use warnings;
use Benchmark qw(cmpthese);

sub commify_regex {
    my $text = reverse $_[0];
    $text =~ s{(\d\d\d)(?=\d)(?!\d*\.)}{$1,}g;
    return scalar reverse $text;
}

sub commify_substr {
    my $v = $_[0];
    my $len = length $v;
    my $dec = index($v, '.');
    my $i = 3 + ($dec < 0 ? 0 : $len - $dec);
    $len -- unless $v == abs($v);
    while ($i < $len ++){
        substr($v, -$i, 0, ',');
        $i += 4;    
    }
    return $v;
}

my @tests = qw(
    1 12 123 1234 12345 123456 1234567 
    12345678 123456789 1234567890 12345678901 
    123456789012 1234567890123
);
push @tests, map "$_.$_", @tests;
push @tests, map - $_,    @tests;

for my $t (@tests){
    print "Incorrect for: ", $t, "\n"
        unless commify_substr($t) eq commify_regex($t);
}

cmpthese( -2, {
    regex  => sub { commify_regex($_)  for @tests },
    substr => sub { commify_substr($_) for @tests },
});

# Output on my Windows machine.

         Rate  regex substr
regex  3277/s     --   -54%
substr 7109/s   117%     --

Answer 6

This can be done with a single regex:

$number =~ s/(?<=\d)(?=(?:\d\d\d)+(?!\d))/,/g;