in reply to Foreach Loop Optimization

That's a scary looking loop :) And scaier HTML...

How much of that 9 seconds is perl runtime, and how much is the time the web browser takes to render the table? Or is that just the perl time? Rendering an 800 row table usually slows browsers down a bit...

There's nothing obviously slow about the way you're doing things.

I suggest doing some experimenting.

What's the speed if you comment out the print statements?

Is FormatTime doing anything stupidly slow?

That's all I can think of offhand...


Mike

Replies are listed 'Best First'.
Re^2: Foreach Loop Optimization
by upallnight (Sexton) on Aug 01, 2007 at 15:45 UTC
    Wow! I didn't expect so many answers so quickly!
    I haven't had time to read them all but I found what was slowing it down so much:
    the print command on its way to the browser
    Here is the FormatTime function code:
    sub FormatTime {
##    Format Time Function    ##
##        Takes Arguments:
##        $time which is the integer value returned by the time functi
+on
##        Returns:
##        a formatted time date string

    my $time = shift;
    (my $sec, my $min, my $hour, my $mday, my $mon, my $year, my $wday
+, my $yday, my $isdst) = localtime($time);
    my $show_min = ":";
    $show_min = ":0" if $min<10;
    $mon++;
    $year += 1900;
    return "$hour$show_min$min $mon/$mday/$year";
}

    [download]

    At first I had shrunk that function down to almost nothing and then thought of the print command.
    Instead of printing inside the loop, I repeatedly appended the output to a string $output and then printed that string outside the loop:
    my $output = "";
            foreach $project_module_list (@project_module_list) {
                if($moduleCount != 0) {
                    @moduleData = split /,/, $project_module_list;
                    $output = $output . "<tr><td align=center><div cla
+ss=table_data>$moduleCount</div></td>";
                    $dataCount = 0;
                    foreach $moduleData (@moduleData) {
                        @moduleField = split /:/, $moduleData;
                        $recentChange = "";
                        $_ = $headers[$dataCount];
                        $localtime = FormatTime($moduleField[2]) if $m
+oduleField[2];
                        $localtime = "n/a" unless $moduleField[2];
                        if( /approv/i ) {
                            $status = "N/A";
                            if($moduleField[0]) {
                                $default = "checked";
                                $status = "$moduleField[1]";
                                $color = " bgcolor=\"#CCFF99\"";
                            }
                            else {
                                $default = "";
                                $color = " bgcolor=\"#FFCC66\"";
                            }
                            if($moduleField[2] > time - $changeBuffer)
+ {
                                $recentChange = " bgcolor=\"#99CCFF\""
+;
                            }
                            $output = $output . "<td$color align=cente
+r><div class=table_data><input type=checkbox name=\"$headers[$dataCou
+nt]$moduleCount\" $default></div></td><td$recentChange title=\"Change
+d by $moduleField[1] at $localtime\"><div class=table_data>$status</d
+iv></td>";
                        }
                        else {
                            if($moduleField[2] > time - $changeBuffer)
+ {
                                $recentChange = " bgcolor=\"#99CCFF\""
+;
                            }
                            $output = $output . "<td$recentChange><div
+ title=\"Changed by $moduleField[1] at $localtime\" class=table_data 
+onClick=\"ToggleShowHide('label_$headers[$dataCount]$moduleCount','$h
+eaders[$dataCount]$moduleCount')\" id=\"label_$headers[$dataCount]$mo
+duleCount\" style='display: block;'>$moduleField[0]</div> <div id=\"$
+headers[$dataCount]$moduleCount\" style='display: none;'><input type=
+text name=\"$headers[$dataCount]$moduleCount\" value=\"$moduleField[0
+]\"><input type=button onClick=\"ToggleShowHide('label_$headers[$data
+Count]$moduleCount','$headers[$dataCount]$moduleCount')\" value=OK></
+div></td>";
                        }
                        $dataCount++;
                    }
                    $output = $output . "</tr>\n";
                }
                $moduleCount++;
                $default = $color = "";
            }
            
            
            print qq~
$output
                </table>
                <script>
                    window.onload=function() { document.approve.submit
+1.disabled=false; }
                </script>
                <table border=0 cellpadding=10><tr>
                    <td>&#160;</td>
                    <td>&#160;</td>
                    <td>&#160;</td>
                    <td><input type=submit value="Save Changes" name="
+submit2"></td>
                </tr></table>
                </form>
            ~;

    [download]

    This changed the run time from an average of 10 seconds to 3 seconds!
    To get the run time I don't do anything fancy, just print the time at the beginning and end of the script.

    Thanks for all the replies!

    http://uan.no-ip.org
[reply]
[d/l]
[select]
      $output = $output . "<td$recentChange><div title=\"Changed by $moduleF
+ie
ld[1] at $localtime\" class=table_data onClick=\"ToggleShowHide('label
+_$headers[$dataCount]$moduleCount','$headers[$dataCount]$moduleCount'
+)\" id=\"label_$headers[$dataCount]$moduleCount\" style='display: blo
+ck;'>$moduleField[0]</div> <div id=\"$headers[$dataCount]$moduleCount
+\" style='display: none;'><input type=text name=\"$headers[$dataCount
+]$moduleCount\" value=\"$moduleField[0]\"><input type=button onClick=
+\"ToggleShowHide('label_$headers[$dataCount]$moduleCount','$headers[$
+dataCount]$
moduleCount')\" value=OK></div></td>";

      [download]
      .... is pretty darned ugly (no offense intended).

      You could tidy that up somewhat, and make it a bit more readable, by doing something like: 

      $output .= qq(
    <td$recentChange>
        <div title="Changed by $moduleField[1] at $localtime"
            class=table_data
            onClick="ToggleShowHide('label_$headers[$d_cnt]$m_cnt','$h
+eaders[$d_cnt]$m_cnt')"
            id="label_$headers[$d_cnt]$m_cnt"
            style='display: block;'>
            $moduleField[0]
        </div>
        <div id="$headers[$d_cnt]$m_cnt"
            style='display: none;'>
            <input type=text name="$headers[$d_cnt]$m_cnt" value="$mod
+uleField[0]">
            <input type=button
                onClick="ToggleShowHide('label_$headers[$d_cnt]$m_cnt'
+,'$headers[$d_cnt]$m_cnt')"
                value=OK>
        </div>
    </td>
    );

      [download]
      Notes:
      • $foo .= $bar is the same as $foo = $foo . $bar
      • use the quoting operators (qq - look for "Regexp Quote-Like Operators" in perlop) in preference to double-quotes - saves lots of messing about escaping special characters.
      • Whilst descriptive variable names are generally considered a good thing™, if you make them too long you wind up with big long statements like the one above, that are difficult to read and understand. Consider shortening some of your variable names - especially those that have limited scope. A good rule of thumb is that the shorter the scope, the shorter the name.
      • When you have a big long statement like the one above, split it over several lines and use indenting for vertical alignment, to improve readability (and make debugging easier).

      Hope this helps,
      Darren :)

[reply]
[d/l]
[select]
        Thanks, I took your advice and cleaned up the readability.

        I still am looking at the html output to see if I can speed up the browser rendering. I took out the javascript and input form fields, but its still taking as long to load the page.
        It must be the table with 750 rows of data thats bogging the browser down.

        http://uan.no-ip.org

[reply]

In Section Seekers of Perl Wisdom