PERL CGI Beginners - RE: Preventing CPU spikes / was switch confusion

Author

RE: Preventing CPU spikes / was switch confusion

David Gilden

2005-04-14, 3:55 am

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__

Charles K. Clarkson

2005-04-14, 8:55 pm

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/;

Sean Davis

2005-04-15, 3:55 am

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

Sponsored Links