Increasing the efficiency of the code

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

Hi, I am using the following code segment to find out the lines which match the given critiria. @array contains the lines from the text file(10000 rows & 90 cols). It takes 10secs to findout the matched records. Is there any ways to increase the performance?.

file:foreach (@array){
       $match=0;
       my @tmparr;
       my @line=split(/\t/);
       foreach $key_pos(sort keys(%conwithposition)) {
               if ($conwithposition{$key_pos}[0] eq '=') {
                  if ($conwithposition{$key_pos}[1] eq $line[$key_pos]
+) {
                     $match=1;
                  }else{
                     $match=0;
                     next file;
                  }
               }elsif($conwithposition{$key_pos}[0] eq '!='){
                  if ($conwithposition{$key_pos}[1] ne $line[$key_pos]
+) {
                     $match=1;
                  }else{
                     $match=0;
                     next file;
                  }
               }elsif ($conwithposition{$key_pos}[0] eq '>'){
                  if ($conwithposition{$key_pos}[1] gt $line[$key_pos]
+) {
                     $match=1;
                  }else{
                     $match=0;
                     next file;
                  }
               }elsif ($conwithposition{$key_pos}[0] eq '>='){
                  if ($conwithposition{$key_pos}[1] ge $line[$key_pos]
+) {
                     $match=1;
                  }else{
                     $match=0;
                     next file;
                  }
               }elsif ($conwithposition{$key_pos}[0] eq '<'){
                  if ($conwithposition{$key_pos}[1] lt $line[$key_pos]
+) {
                     $match=1;
                  }else{
                     $match=0;
                     next file;
                  }
               }elsif ($conwithposition{$key_pos}[0] eq '<='){
                  if ($conwithposition{$key_pos}[1] le $line[$key_pos]
+) {
                     $match=1;
                  }else{
                     $match=0;
                     next file;
                  }
               }
          }
      if ($match){ #All the conditions met. Push this line of file to 
+@result_arr
          foreach $col(sort {$a<=>$b} keys(%cols_pos)){
              push @tmparr,$line[$col];
          }
          $tmparr=join("|",@tmparr);
          push @listcols,$cols_pos{$col};
          push @result_arr,[ $tmparr ];
      }
}#file:for
[download]

Thanks in Adv,

Comment on Increasing the efficiency of the code Download Code

Replies are listed 'Best First'.
Re: Increasing the efficiency of the code by Limbic~Region (Chancellor) on Nov 29, 2006 at 13:08 UTC
Anonymous Monk, I have re-written your code ~~to be~~ to possibly be much more efficient. I have not provided comments so I suspect you won't immediately understand it. The strict and warnings pragmas can point out a lot of problems in your code and as such should be used. my @sorted_keys = sort keys %conwithpostion; my @sorted_cols = sort {$a <=> $b} keys %cols_pos; my %dispatch = ( '=' => sub {$_[0] eq $_[1]}, '!=' => sub {$_[0] ne $_[1]}, '>' => sub {$_[0] gt $_[1]}, '>=' => sub {$_[0] ge $_[1]}, '<' => sub {$_[0] lt $_[1]}, '<=' => sub {$_[0] le $_[1]}, ); for (@array) { my $match = 0; my @line = split /\t/; for my $key_pos (@sorted_keys) { my ($op, $arg1, $arg2) = (@{$conwithposition{$key_pos}}[0, 1], + $line[$key_pos]); $match = $dispatch{$op}->($arg1, $arg2); last if ! $match; my $col = $sorted_cols[-1]; push @listcols, $col; push @result_arr, join '\|', @line[@sorted_cols]; } } [download] Please feel free to ask questions, but try to find the answers on your own first. See Coping with Scoping and perldoc (also available from the command line) for starters. The trick I did to avoid the if/eslif chain is called a dispatch table. Update: As GrandFather points out in his node, your use of $col is likely incorrect. I set $col to the last value of the sorted %cols_pos keys on the assumption you thought that is the value it would be after the loop. If $col is set elsewhere in your code then you will need to change this accordingly. Cheers - L~R	[reply] [d/l]
Re: Increasing the efficiency of the code by GrandFather (Saint) on Nov 29, 2006 at 12:39 UTC
For a start take the two sorts on invariant hashes (%conwithposition and %cols_pos) outside the main loop. Perform the sorts once each and assign the results to arrays. You should `use strict; use warnings;`. It looks like `push @listcols, $cols_pos{$col};` is bogus: $col is not defined at that point unless it has been set elsewhere in the program. The $col used as a loop variable in the preceeding foreach is not the same $col used as a key. A small amount of data and sample contents for the hashes may help evaluate the benefits of other alterations to the code. In this case a little bench marking may help, but code that won't run can't be bench marked. DWIM is Perl's answer to Gödel	[reply] [d/l] [select]
Re: Increasing the efficiency of the code by madbombX (Hermit) on Nov 29, 2006 at 12:32 UTC
The best thing to do is to do some performance profiling. Check out some of these nodes on profiling: Re: Improving the efficiency of code when processed against large amount of data or performance profiling. In these nodes there are discussions on profiling your code line by line. Figure out which line takes the longest to process and try and bring down that time.	[reply]
Re: Increasing the efficiency of the code by Fengor (Pilgrim) on Nov 29, 2006 at 13:04 UTC
I don't know if it would actually increase performance but since the whole thing seems to be about wrapping from numerical to string context that gave me an idea. If you implemented a hash with the numerical operators as keys and the string operators as their corresponding values couldn't you rewrite the whole if elsif else etc construct as: `if (eval "$conwithposition{$key_pos}[1] $mapContext{$conwithposition{$ +key_pos}[0] $line[$key_pos]"){ foreach $col(sort {$a<=>$b} keys(%cols_pos)){ push @tmparr,$line[$col]; } $tmparr=join("\|",@tmparr); push @listcols,$cols_pos{$col}; push @result_arr,[ $tmparr ];` [download] -- "WHAT CAN THE HARVEST HOPE FOR IF NOT THE CARE OF THE REAPER MAN" -- Terry Pratchett, "Reaper Man"	[reply] [d/l]
Re^2: Increasing the efficiency of the code by Limbic~Region (Chancellor) on Nov 29, 2006 at 13:14 UTC
Fengor, If you implemented a hash with the numerical operators as keys and the string operators as their corresponding values couldn't you rewrite the whole if elsif else etc construct as: You have a good idea and yes, it will lead to a performance increase. Unfortunately your implementation is less than desireable. Using a string eval can be very dangerous and should be avoided when better alternatives exist. In this case, a dispatch table is better suited. See my post below for an example. Cheers - L~R	[reply]
Re^3: Increasing the efficiency of the code by Fengor (Pilgrim) on Nov 29, 2006 at 13:20 UTC
hmm thx, at least tis good to know i wasnt totally on the wrong track. That dispatch table looks good guess its about time to add that trick to my book ;) -- "WHAT CAN THE HARVEST HOPE FOR IF NOT THE CARE OF THE REAPER MAN" -- Terry Pratchett, "Reaper Man"	[reply]