RE: Preventing CPU spikes / was switch confusion

Dear Perl Gurus,

I have some  problems that I think are a result of how my Switch statement =
is written.=20
This script is invoked via a web browser to upload a file, and do a few oth=
er things.=20
However it appears that the user system / network, or my script is stalling=
=2E So
the user clicks the button  a second and a third time before the script has
finished. The system admin has made me aware that this script is maxing out=
the
CPU usage of server. He is not happy....


I thought have surrounding a portion of the code and putting in an if=20
block ...

if (!$state) {

do lots of stuff related to file upload...
}   =20

$state could be read and written to and store a 0 for ready to do something=
,=20
or a 1 for, 'busy now, don't brother me'......


Another issue to improve the performance of the script
was to move stuff inside a switch block, but I am not sure how this would=
=20
impact scope :=20

SWITCH: {
=20
if  ($action =3D~ /Upload/) {
=20

use Fcntl qw( :DEFAULT :flock );
use constant MAX_FILE_SIZE  =3D> 2 * 1_048_576;   # Limit each upload to 2 =
MB
use constant MAX_DIR_SIZE   =3D> 10 * 1_048_576; # Limit total uploads to 1=
0 MB
use constant MAX_OPEN_TRIES =3D> 100;
last SWITCH;   =20
};

--

if  ($action =3D~ /AnotherCmd/) {
### is order backwards on these next two lines?
exit;
last SWITCH;=20
}


Could some comment on using Switch or Case statements vers If {} else block=
s

Thanks...
Dave Gilden
PS: I am not sure of syntax that I should be using for Switch I have seen s=
everal variants=20
as I have read documentation today....


--- what I have now----

use CGI qw/:standard/;
use CGI;
use Fcntl qw( :DEFAULT :flock );
use CGI::Carp qw(fatalsToBrowser);
use strict;
use switch;=20


my $action =3D $q->param( "action" );

SWITCH: {
=20
if  ($action =3D~ /Upload/) {
last SWITCH;   =20
};
=20
if  ($action =3D~ /Update/) {
print redirect("./import_clean_csv.php");
exit;
last SWITCH;   =20

};
=20
=20
if  ($action =3D~ /Clean/) {
my @filesToRemove; =20
=20
chdir UPLOAD_DIR or die "Couldn't chdir to _data directory: $!";
=20
opendir(DR,"./");
@filesToRemove  =3D  grep  {$_ =3D~ /^(\w[\w.-]*)/} readdir DR;
closedir DR;


print $HTML_HEADER;
print '<div align=3D"center">';


foreach my $fr (@filesToRemove) {
=20
print  "Deleted $fr<br>\n";
unlink($fr)  or die "Couldn't Delete $fr $!";
}



print <<HTML_OUT;
<p class=3D"top-header">Your Done close this window!=20
<form><input type=3D"button" onclick=3D"self.close()" value=3D"Close  Windo=
w"></form></p>
</div>
HTML_OUT
print end_html;

exit;
last SWITCH;=20
};
}


#more....

__END__

David Gilden <mailto:dowda@coraconnection.com> wrote:

: --- what I have now----
:
: use CGI qw/:standard/;
: use CGI;

Why use CGI.pm twice? You need to read the CGI.pm docs. There
is no reason to ever call the module more than once.


: use Fcntl qw( :DEFAULT :flock );
: use CGI::Carp qw(fatalsToBrowser);
: use strict;

Also need to turn on warnings.

use warnings;   # or use -w switch on older perl versions


: use switch;

CPAN has only Switch.pm, not switch.pm. Did you create your
own module or is this a typo?


: my $action = $q->param( "action" );

You have not yet defined $q as a CGI object. You should be
seeing this error in your browser.


: SWITCH: {

This does not follow the Switch.pm docs. It's basically a
named block which allows you to avoid writing 'else' clauses.


:     if  ($action =~ /Upload/) {
:         last SWITCH;
:     };
:
:     if  ($action =~ /Update/) {
:         print redirect("./import_clean_csv.php");
:         exit;
:         last SWITCH;

Too late. You have already exit()ed the script.


:     };
:
:
:     if  ($action =~ /Clean/) {

Indent the same way throughout the whole script. The previous
'if' blocks were indented four spaces. Why not do the same here?


:     my @filesToRemove;

Whenever possible, declare your variables as you first use
them.


:     chdir UPLOAD_DIR or die "Couldn't chdir to _data directory: $!";

UPLOAD_DIR has not been defined in this script. You should be
seeing this error in your browser.


:     opendir(DR,"./");

How do you know it opened?

opendir DR, './' or die qq(Cannot open "./": $!);



:     @filesToRemove  =  grep  {$_ =~ /^(\w[\w.-]*)/} readdir DR;

my @filesToRemove = grep {$_ =~ /^(\w[\w.-]*)/} readdir DR;


:     closedir DR;
:
:
: print $HTML_HEADER;

$HTML_HEADER is not defined in this script. You should be
seeing this error in your browser.


: print '<div align="center">';
:
:
:     foreach my $fr (@filesToRemove) {
:
:     print  "Deleted $fr<br>\n";
:     unlink($fr)  or die "Couldn't Delete $fr $!";
:     }
:
:
:
: print <<HTML_OUT;
: <p class="top-header">Your Done close this window!
: <form><input type="button" onclick="self.close()"
:        value="Close  Window"></form></p>
: </div>
: HTML_OUT
: print end_html;
:
:         exit;
:         last SWITCH;

Too late. You have already exit()ed the script.



:     };
: }
:
:
: #more....

Other than typos and uninitialized variables, nothing above
seems to be hogging resources. The problem is likely in this
unseen part or in the switch.pm module (if that wasn't a typo.)


: __END__


How about skipping the Switch stuff and using something like this.

use CGI qw/:standard Button/;

On Apr 14, 2005, at 2:09 PM, Charles K. Clarkson wrote:
>
>     How about skipping the Switch stuff and using something like this.
>
> use CGI qw/:standard Button/;
>     .
>     .
>     .
>
> if ( param() ) {
>     if ( param( 'action' ) =~ /Upload/ ) {
>         # call upload sub
>
>     } elsif ( param( 'action' ) =~ /Update/ ) {
>         print redirect("./import_clean_csv.php");
>
>     } elsif ( param( 'action' ) =~ /Clean/ ) {
>
>         print
>
>         header(),
>             start_html( -title => 'Clean Upload Directory' ),
>
>                 div( { align => 'center' },
>
>                     ul( { style => 'list-style-type: none;' },
>                         li( clean_dir( 'upload_dir' ) ),
>                     ),
>
>                     Button( { onclick => 'self.close()' },
>                         'Close Window'
>                     ),
>                 ),
>             end_html();
>
>     }
>
> } else {
>     # do other stuff
> }
>
> #more ...
>
>
> sub clean_dir {
>     # Not Tested
>
>     # Don't output anything to the browser from this subroutine.
>     # Return an array reference containing the report. Allow the
>     # caller to decide how it will be marked up.
>
>     my $upload_dir = shift;
>     chdir $upload_dir or die qq(Couldn't chdir to "$upload_dir": $!);
>
>     opendir my $dir, './' or die qq(Cannot open "./": $!);
>     my @filesToRemove = grep {$_ =~ /^(\w[\w.-]*)/} readdir $dir;
>     closedir $dir;
>
>     my @report;
>     foreach my $file ( @filesToRemove ) {
>         if ( unlink $file ) {
>             push @report, qq(Deleted "$file");
>
>         } else {
>             push @report, qq(Could not delete "$file": $!);
>         }
>
>     }
>
>     return \@report;
> }
>
> __END__
>

Or look at one of the several different modules designed to simplify
the process of creating a multipage script like CGI::Application,
CGI::Builder, or CGI::FormBuilder (somewhat).

Sean