How to optimize this code

avo has asked for the wisdom of the Perl Monks concerning the following question:

Dear monks,
How do you think I can optimize this code:

my @allpixels = split( /,/ , $pixellist );
my @rgbvals;

foreach my $pixel (@allpixels) {
  push @rgbvals, ($pixel >> 16 & 255, $pixel >> 8 & 255, $pixel & 255)
+;
}

my $newblob = pack "C*", @rgbvals;
[download]

In this example my first list is:
(9347248,9347248,9413041)
and second list is:
(142,160,176,142,160,176,143,161,177) which represents:
(R,G,B,R,G,B,R,G,B)
I was thinking of using unpack() instead of $pixel >> 16 & 255, $pixel >> 8 & 255, $pixel & 255
Do you think that unpack will perform better?

Comment on How to optimize this code Select or Download Code

Replies are listed 'Best First'.
Re: How to optimize this code by moritz (Cardinal) on Sep 10, 2008 at 17:09 UTC
Do you think that unpack will perform better? Why don't you just try? If you can measure the speed difference, that's fine. If not, it's so small that it doesn't matter for you.	[reply]
Re: How to optimize this code by Fletch (Bishop) on Sep 10, 2008 at 17:15 UTC
Seconded on the benchmark first suggestion. Baring that it'd be trivial to memoize the converted values if there's a high likelyhood that they'll show up repeatedly (then again you're wagering that the hash lookup and deref will be faster than 3 shifts and bitwise-ands, so again benchmark). `my %_pixel_as_rgb; for my $pixel ( @allpixels ) { push @rgbvals, @{ $_pixel_as_rgb{ $pixel } \|\|= [ $pixel >> 16 & 255, $pixel >> 8 +& 255, $pixel & 255 ] }; }` [download] The cake is a lie. The cake is a lie. The cake is a lie.	[reply] [d/l]
Re: How to optimize this code by jwkrahn (Abbot) on Sep 10, 2008 at 21:04 UTC
See if this is better: `my $newblob = join '', map substr( pack( 'N', $_ ), 1 ), $pixellist =~ + /\d+/g;` [download] Update: or maybe this: `( my $newblob = pack 'N*', $pixellist =~ /\d+/g ) =~ s/.(...)/$1/sg;` [download]	[reply] [d/l] [select]
Re: How to optimize this code by betterworld (Curate) on Sep 10, 2008 at 19:44 UTC
`my $pixellist = '9347248,9347248,9413041'; my @allpixels = split( /,/ , $pixellist ); my $newblob = reverse pack "(VX)*", reverse @allpixels;` [download] Well, I'm not suggesting that this is faster than the original or that you use it; I just wanted to prove that you can omit the entire for-loop if you make your pack command sufficiently unreadable ;-)	[reply] [d/l]