Re: Pls help optomize - ksh version 60% faster!

Here's a version that (shouldn't) change the functionality, but cleans up some the code with "more perlish" code .. especially the regex usage (use $x =~ /foo/ instead of assigning to $_) and some of the if-else logic (most notably using grep). Diff against your original to see all the individual differences .. But hopefully this will shorten and clarify the code so it's easier for you to go from there...

#! /usr/local/bin/perl
use warnings;
use strict;

########################## Execution Setup ###########################
+###########
use Cwd;
# Local Modules

######################################################################
+###########
# File: $PWD/gcenrollrpt                                              
+          #
# Purpose: Retrieve logs creates a daily report of various gc statisti
+cs        #
# Usage:   gcenrollrpt  date YYYY-MM-DD                               
+          #
# Notes:                                                              
+          #
######################################################################
+###########
system("date");

# Command Line Syntax Checking
if ( (! $ARGV[0] ) ) {
   print "\nUSAGE: $0 <date>\n";
   exit 1;
}

# set primary vars
my $today = `date +%Y-%m-%d`;
chomp($today);
my $date = $ARGV[0];
chomp($ARGV[0]);
my $host = `uname -n`;
chomp($host);
my @banks = qw[ 321 920 144 ];

# Read in statements to be searched for in log from ini file
my $iniFile = "$ENV{'PWD'}/gcenrollrpt.ini";
open(INIFILE, $iniFile) or die "Can't open $iniFile $!\n";
my @entries = <INIFILE>;

# Determine which log to read
my ($logfile, @logfile);
my $rmfile=0;
my $archive = "/log_archive/GC/$host/GC/logs/GC.log.$date.gz";
if ( -e "/GC/logs/GC.log.$date" )
{
   $logfile = "/GC/logs/GC.log.$date";
}
elsif ( -e $archive )
{
   system("gzcat $archive >  /GC/logs/GC.log.$date" );
   $logfile = "/GC/logs/GC.log.$date";
   $rmfile = 1;
}
else
{
   $logfile = "/GC/logs/GC.log";
}

# Parse the log
my (%counts, @rptType);
open( LOGFILE, $logfile ) or die "Can't open file $logfile $!\n";
LOG:while ( my $logEntry = <LOGFILE> )
{
   next LOG unless $logEntry =~ /^ $date/;
   foreach my $bank (@banks)
   {

      # check each entry from the ini file against the log
      ENTRY:foreach my $entryTemplate ( @entries )
      {
           # split the entry type from the actual verbage to be search
+ed for
           chomp($entryTemplate);
           next unless length($entryTemplate);

            my ( $rptType, $entry ) = $entryTemplate =~ /(.+)~(.+)/;

            # make the search bank specific
            $entry =~ s/\$bank/$bank/g;
            next unless length($entry) && length($rptType);

            # set up counters for each type and store unique types for
+ each bank
            # serves as an index when outputting results
#            push @rptType, $rptType unless @rptType;    # UPDATE -- r
+emove this line -- it's unnecessary
            push @rptType, $rptType unless grep /$rptType/, @rptType;

            # set up the key $typeHour for the %counts var that stores
+ all the totals
            # must be done first to set up counts for entries which th
+ere are no matches
            my $hour =   $logEntry =~ /\d{4}-\d{2}-\d{2}\s(\d{2}):/;
            my $typeHour = "$bank-$rptType-$hour";

            # Process Matching entries
            $counts{$typeHour} ||= 0;
            $counts{$typeHour}++ if $logEntry =~ $entry;
      }
   }
}
close (LOGFILE);

# Remove the log file if unzipped from the archive
system("/usr/bin/rm", "-f", $logfile) if ( $rmfile == 1 );

# Create reports by bank
foreach my $bank (@banks)
{
   my $tmpFile1 = "/tmp/gcenrollrpt$bank.1.$$";
   open ( TMPFILE1, ">", $tmpFile1 ) or die "Can't open $tmpFile1 $!\n
+";
   foreach my $type (@rptType)
   {
      foreach my $i ( 0..23 )
      {
         foreach my $typeHour ( sort keys %counts )
         {
            my ($bnk,$rptType,$hr) = $typeHour =~ /(.+)-(.+)-(.+)/;
            my $hour = sprintf "%02d", $i;
            next unless "$bank-$type-$hour:" =~ /$bnk-$rptType-$hr:/;
            print TMPFILE1 ( $i ? "$hour: $counts{$typeHour}\n" : "\n\
+n$type\n" );
         }
      }
   }
   close(TMPFILE1);

   # Mail the report out
   my $cmdLine = "cat $tmpFile1 | mailx -s \"OAC report for bank $bank
+ $date $host\" xxx\@xyz.com";
   system("$cmdLine");
   system("/usr/bin/rm -f $tmpFile1");
}

system("date");

0;
[download]

Comment on Re: Pls help optomize - ksh version 60% faster! Select or Download Code

Replies are listed 'Best First'.
Re^2: Pls help optomize - ksh version 60% faster! by Skeeve (Parson) on Sep 28, 2005 at 22:48 UTC
Question: instead of `push @rptType, $rptType unless @rptType; push @rptType, $rptType unless grep /$rptType/, @rptType;` [download] I would use `push @rptType, $rptType unless $seen{$rptType}++;` [download] Don't you think this would be faster than the grep, besides the fact that (I think) a grep for $rptTyp being 'a' would match a stored $rptType of 'XaX'. I know! The OP had that code and maybe it's exactly what the OP intended, but it "smells fishy" ;-) `$\=~s;s.;q^\|D9JYJ^^qq^\//\\\///^;ex;print`	[reply] [d/l] [select]
Re^3: Pls help optomize - ksh version 60% faster! by davidrw (Prior) on Sep 29, 2005 at 01:29 UTC
Yes, you're right -- the hashing would be more efficient, though `@rptType` is used later in the code, so need to keep `%seen` in addition, or the later use might be able to be `keys %seen` instead of `@rptType` (unless order matters) .. or could just always push onto @rptType and throw out dups later (see tihs idiom, but destroys order) As for matching with `/$rptType/`, y, that seems suspicious .. i too now suspect that it should be `eq` instead, but can't say for sure.. also, looking at this again, the first of my two lines is unnecessary -- it seemed at first that the author wanted a double `push` onto @rptType, but that's not the case.. I've ammended my post above.. anyways, ++ and thanks for double-checking my post!!	[reply] [d/l] [select]


Problems? Is your data what you think it is?
	PerlMonks