in reply to Re: Re: Shell Script Woes (tye's try)
in thread Shell Script Woes

First, I'd use more than one space for indentation. I use 4 because I like the way it discourages overly deep nesting of code. Even 2 or 3 would be quite a bit better than 1, IMO.

$Traps{"$Fields[6]"} = [ $Expiration,$Fields[2],$Fields[5],$Fields[7] ]; can be written $Traps{$Fields[6]} = [ $Expiration, @Fields[­2,5,7] ]; Putting in too many quotes can bite you (though using it as a hash key also does the stringification which would bite you in the same way in this case -- changing an object into a string) so be careful of it.

You can make the code clearer using a few constants: 

    sub EXPIRE() { 0 }
    sub WHATEVER() { 1 }
    sub FOOBAR() { 2 }
    sub FILENAME() { 3 }

[download]
(these make no difference in the running time of the code since they get optimized away at compile time). I'd also avoid 'unless' so push @GrepList,$Traps{$tr­ap}[3] unless (($Traps{$trap}[0] < $Now && $Traps{$trap}[1]) || $trap eq "SIZE"); becomes 
push @GrepList, $Traps{$tr­ap}[FILENAME]
    if  $trap ne "SIZE"
   and     ! $Traps{$trap}[WHATEVER]
        || $Now <= $Traps{$trap}[EXPIRES];

[download]
for example (I find spacing more effective at conveying grouping than parens, YMMV).

Don't use map unless you want the list that it builds: map { $MaxLen = length($_) if length($_) > $MaxLen } @GrepList; becomes 

for(  @GrepList  ) {
    $MaxLen = length($_) if length($_) > $MaxLen;
}

[download]
If you really have a need for single-line code (which is a mistake in my book), then remove the newlines.

Use local( $/ )= \$BufferSize; and you can drop the $/ = "\n"; line.

So no speed improvements to offer. (:

                - tye

Replies are listed 'Best First'.
Re: Re^3: Shell Script Woes (review)
by Limbic~Region (Chancellor) on Jan 10, 2003 at 23:14 UTC
    I took a few of your suggestions in formatting, but left some as they were

    The problem I found is the more elements in the GrepString, the longer it took to process (exponential).

    I tore the modified tcgrep from the The Unix Reconstruction Project apart to find out how it was so quick. When I combined that code with the buffer stuff - it started screaming.

    Here is the latest version - I know better than to think it is the last 

    #!/usr/bin/perl -w
use strict;
use Time::Local;
chdir "/var/spool/wt400/gateways/$ARGV[0]";
mkdir "capture", 0755 unless (-d "capture");
my $Dir = $ARGV[1];
my $ListTime = 0;
my $BufferSize = 64 * 1024;
my $MaxLen = 0;
my %Traps;
my %Vals;
my @GrepList;
my $Counter = 1;
my $Size;
my $Prev;
my $Now;
my $NF;
my $Matches;
my $matcher;
my $match_code;
my $String;

while (1) {
  if ($Counter > 20 || ! @GrepList) {
    if ( (stat("traplist.$Dir"))[9] gt $ListTime ) {
      $ListTime = (stat(_))[9];
      open (LIST,"traplist.$Dir");
      while (<LIST>) {
        next if ($_ =~ /^Created\t\tExpires/ || $_ =~ /^ *$/);
        my @Fields = split "\t" , $_;
        my($mon, $day, $year, $hour, $min) = split ?[-/:]? , $Fields[1
+];
        my $Expiration =  timelocal(0, $min, $hour, $day, $mon - 1, $y
+ear + 100);
        $Traps{"$Fields[6]"} = [ $Expiration,@Fields[2,5,7] ];
      }
      close (LIST);
    }
  }

  @GrepList = ();
  $Now = time;

  foreach (keys %Traps) {
    push @GrepList,$Traps{$_}[3] unless (($Traps{$_}[0] < $Now && $Tra
+ps{$_}[1]) || $_ eq "SIZE");
    $Vals{$Traps{$_}[3]} = $_;
  }

  foreach (@GrepList) {
   $MaxLen = length($_) if length($_) > $MaxLen;
   $_ =~ s/(\W)/\\$1/g;
   $_ = "(?i-xsm)$_";
   $match_code .= "(\$Matches = \"$_\") =~ s/\\(\\?i-xsm\\)// if \$Str
+ing =~ /$_/;";
  }

  if (exists $Traps{"SIZE"} && $Traps{"SIZE"}[1]) {
    $Size = $Traps{"SIZE"}[2] unless ($Traps{"SIZE"}[0] < $Now && $Tra
+ps{"SIZE"}[2] > 0);
  }

  exit unless (@GrepList || $Size);

  $matcher = eval "sub { $match_code }";

  if ($Dir eq "out") {
    @ARGV = <out/do*>;
  }
  elsif ($Dir eq "in") {
    @ARGV = <in/di*>;
  }
  else {
    @ARGV = <out/do* in/di*>
  }

  matchfile($matcher,@ARGV);
  ++$Counter;
  sleep 3
}

sub matchfile {
  $matcher = shift;
  my $file;
  my $name;
  local($_);
  local($/) = \$BufferSize;
  FILE:
  while (defined ($file = shift(@_))) {
    if ($Size && (stat("$file"))[7] >= $Size) {
      ($NF = "$file-SIZE") =~ s/^.*\///;
      rename $file , "capture/$NF";
      next FILE;
    }
    unless (open(FILE, $file)) {
      next FILE;
    }
    $Matches = 0;
    $Prev = "";
    while (<FILE>) {
      $_ =~ tr/\n//d;
      $String = $Prev . $_;
      &{$matcher}();
      if ($Matches) {
        ($NF = "$file-$Vals{$Matches}") =~ s/^.*\///;
        rename $file , "capture/$NF";
        $Prev = "";
        next FILE;
      }
      if (eof) {
        $Prev = "";
      }
      else {
        $Prev = substr($_,-$MaxLen);
      }
    }
  }
}

    [download]
    Any comments (especially in the way of speed improvements) will be greatly appreciated!

    L~R

[reply]
[d/l]

In Section Seekers of Perl Wisdom