Strange slowdown 
Author Message
 Strange slowdown

I was going through some code, looking for a slowdown, and I came to
this loop which is relatively *crawling* - does anyone see any
potential problems here?  There are other loops like this, looking at
the file structure and incorporating it into the data structure.  I'm
just looking for anything glaring.  Also, I found that it saves about
.01 sec per iteration to set a filehandle directly to an array rather
than loop through the filehandle and push onto the array - is this an
interpreter issue?

opendir(MAPS,".") || die("Couldn't open directory $DATA_DIR{'Map'}:
$!");
foreach $Map (read_directory(\*MAPS)) {
        $Self->{"Map"}->{$Map} = {};
        opendir(PARTS,"$Map") || die("Couldn't open directory
$DATA_DIR{'Map'}$Map: $!");
        foreach $Part (read_directory(\*PARTS)) {
                open(MAP,"$Map/$Part") || die("Couldn't open file
$DATA_DIR{'Map'}$Map/$Part: $!");


                close(MAP);

        }
        closedir(PARTS);

Quote:
}

closedir(MAPS);

John Dutton



Tue, 23 Dec 2003 05:18:01 GMT  
 Strange slowdown

Quote:

> I was going through some code, looking for a slowdown, and I came to
> this loop which is relatively *crawling* - does anyone see any
> potential problems here?  There are other loops like this, looking at
> the file structure and incorporating it into the data structure.  I'm
> just looking for anything glaring.  Also, I found that it saves about
> .01 sec per iteration to set a filehandle directly to an array rather
> than loop through the filehandle and push onto the array - is this an
> interpreter issue?

> opendir(MAPS,".") || die("Couldn't open directory $DATA_DIR{'Map'}:
> $!");

You open "." but complain about not being able to open $DATA_DIR{'Map'};
if the current directory is not, in fact, $DATA_DIR{'Map'}, then this
error message is wrong.  Also, I don't see anything localizing the MAPS
glob... you should either put on the prior line "local *MAPS;" or else
use a lexical for the dirhandle.  Also, since $DATA_DIR{Map} is used so
often in the following code, I would advise you assign the value to some
variable.

        my $mapdir = $DATA_DIR{Map};
        opendir(my $maps, $mapdir)
                or die "Couldn't open directory $mapdir: $!";

Quote:
> foreach $Map (read_directory(\*MAPS)) {

Any particular reason you don't simply use readdir?  Just to exclude "."
and ".."?  Also, where is $Map declared?  Is it a dynamic [global] or
lexical variable?  For good style, always give variables the smallest
scope they need.
        foreach my $map (grep !/^\.\.?$/, readdir($maps)) {

Hmm, since $maps isn't needed beyond this statement, we could reduce its
scope even further:

        my $mapdir = $DATA_DIR{Map};
        for my $map (do {
                opendir(my $maps, $mapdir)
                        or die "Couldn't open directory $mapdir: $!";
                grep !/^\.\.?$/, readdir($maps) } ) {

Since $maps is lexically scoped, it is automatically closed at the end
of the enclosing block (the do block, in this case).

Quote:
>         $Self->{"Map"}->{$Map} = {};

Just style, but the "" aren't needed here, nor is the second ->
        $Self->{Map}{$map} = {};
Actually, this assignment isn't needed at all, due to autovivification.

Quote:
>         opendir(PARTS,"$Map") || die("Couldn't open directory
> $DATA_DIR{'Map'}$Map: $!");
>         foreach $Part (read_directory(\*PARTS)) {
>                 open(MAP,"$Map/$Part") || die("Couldn't open file
> $DATA_DIR{'Map'}$Map/$Part: $!");


>                 close(MAP);

>         }
>         closedir(PARTS);
> }
> closedir(MAPS);

Neither PARTS nor MAP are localized... Also, the same things which were
done for MAPS could be done for these.

my $mapdir = $DATA_DIR{Map};
for my $map (do {
        opendir(my $maps, $mapdir)
                or die "Couldn't open directory $mapdir: $!";
        grep !/^\.\.?$/, readdir($maps) })
{
        $Self->{Map}{$map} = {}; # this line not needed?
        for my $part (do {
                opendir( my $parts, "$mapdir/$map" )
                        or die "Couldn't opendir $mapdir/$map: $!";
                grep !/^\.\.?$/, readdir $parts })
        {
                open my $image, "<", "$mapdir/$map/$part"
                        or die "Couldn't open $mapdir/$map/$part: $!";

        }

Quote:
}

Due to scoping, each of the opened filehandles is implicitly closed at
just the right place.  NB This code hasn't been tested.  If it doesn't
work, I would suggest you try and learn why it doesn't work (eg, older
perls may not autovivify filehandles).

--
The longer a man is wrong, the surer he is that he's right.



Wed, 24 Dec 2003 11:16:14 GMT  
 
 [ 2 post ] 

 Relevant Pages 

1. installation problem!

2. DB browsers

3. G.L.A.D. components

4. help for including units in main file (i am using BP7)

5. NEW in InterBase

6. inconsistent filtering problem

7. Slowdown when calling system() twice

8. memory leak and slowdown in Perl 5 alpha 12e

9. Associative array slowdown?

10. Dramatic slowdown when processing files bigger than 1Gb

11. IO::Socket Slowdown

12. Strange problem: strange character added during a mkdir

 

 
Powered by phpBB® Forum Software