Beefy Boxes and Bandwidth Generously Provided by pair Networks
Don't ask to ask, just ask
 
PerlMonks  

Increasing the efficiency of the code

by Anonymous Monk
on Nov 29, 2006 at 12:16 UTC ( #586667=perlquestion: print w/replies, xml ) Need Help??
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
Thanks in Adv,

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]; } }
    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

Re: Increasing the efficiency of the code
by GrandFather (Sage) 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
Re: Increasing the efficiency of the code
by madbombX (Hermit) on Nov 29, 2006 at 12:32 UTC
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 ];

    --
    "WHAT CAN THE HARVEST HOPE FOR IF NOT THE CARE OF THE REAPER MAN"
    -- Terry Pratchett, "Reaper Man"

      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

        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"

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://586667]
Approved by Joost
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others romping around the Monastery: (5)
As of 2018-09-20 18:12 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Eventually, "covfefe" will come to mean:













    Results (178 votes). Check out past polls.

    Notices?
    • (Sep 10, 2018 at 22:53 UTC) Welcome new users!