Re: script hangs up when reading in file

I think this is memory related... The files are big and your keeping all your data in a big string... I would definitely recommend not using the foreach loops to loop thru the file; I think perl will try to initialize the loop values first, which means it will try to read the entire file into a temporary variable... As I understand it the files can be quite large.

I would replace the file reading loops with while(my $line=<DHCPDCONF>) and while(my $line=<LEASES>) respectively. You could even do just while(<DHCPDCONF>) and drop the line$=~ parts from your regex's. This will save a lot of memory and also make the script faster. You could save the information you'll need to rewrite the config file in a hash which is faster and cheaper on your memory.

In the same spirit I would place the print DHCPDCONF line into the loop that generates the file content, and replace the loop with: while(my($device,$ip)=each %devices) and use $ip in the substitution regex. This again saves a lot of memory and is faster. I you really want to handle the existing file content, you could loop thru the file line by line, extract it's $devices key from the line, check to see if it exists in your hash with ip's, and if so perform a substitution on the line. All the while saving to a temporary file, which you later move over the original.

This should be faster, cleaner and ultimately more versatile. (And it might not hang...)

Comment on Re: script hangs up when reading in file Select or Download Code

Replies are listed 'Best First'.
Re: Re: script hangs up when reading in file by tombmbdil (Beadle) on Jan 17, 2003 at 17:13 UTC
I hadn't realized that while loops were more efficient than for loops... I just liked the english-like syntax of the foreach loop :) I also usually try to avoid the $_ variable because early in my perl career I always had trouble keeping track of when it was modified. Anyway, stylistic preferences aside... I made the foreach -> while loop changes you suggested and am getting the same results. With both the original and modified versions of the code the hangup point has moved to just after Marker 2. I also decided to leave things alone and let the code run its course... and it does decide to continue after about 30 seconds. Which might at least be acceptable as this script will only be running as an hourly cronjob. I'm also not sure if this really is a memory issue. Here's the output from top while the script is hung up: `PID COMMAND %CPU TIME #TH #PRTS #MREGS RPRVT RSHRD RSIZE VSIZE 1279 perl 19.6% 0:02.28 1 10 21 524K 1.42M 1.62M 2.92M` [download] As you can see, perl isn't using up much memory at all. For those of you wondering about the strange top format... this is on my ibook. Here's the output from top on a linux box: `PID USER PRI NI SIZE RSS SHARE STAT %CPU %MEM TIME COMMAND 16974 root 25 0 1696 1696 1128 R 51.5 0.1 0:02 ip_update.pl` [download] Well, in the middle of writing this I finally understood what you meant by movind the print DHCPDCONF line and the temporary file... so I made those adjustments and, of course there is no hang. I'm still not sure why it was hanging in the first place, but I guess that is a moot point now. Thanks for your help :) Updated Code: #!/usr/bin/perl use strict; ##################################################### # Update MAC-IP pairings for all registered devices # ##################################################### my %devices; # Open dhcpd.conf and initialize the %devices hash with all # registered modems and cpes open (DHCPDCONF, "dhcpd.conf") or die "Cannot read from file: $!"; while (<DHCPDCONF>) { $devices{lc($1)} = '000.000.000.000' if (/#.#.#.# Modem MAC: (\w+:\w+:\w+:\w+:\w+:\w+)/ or /subclass "." 1:(\w+:\w+:\w+:\w+:\w+:\w+)/); } close DHCPDCONF; # open leases file and match mac addresses in %devices with active ips open (LEASES, "dhcpd.leases"); my ($current_ip, $state); while (<LEASES>) { $current_ip = $1 and next if /^\slease (\d+.\d+.\d+.\d+)/; $state = $1 and next if /^\sbinding state (\w+)/; $devices{lc($1)} = $current_ip and print lc($1) . " - " . $devices{lc($1)} . "\n" and next if (/^\shardware ethernet (\w+:\w+:\w+:\w+:\w+:\w+)/ and $state eq 'active' and exists $devices{lc($1)}); } close LEASES; # substitute old ip with current value open (DHCPDCONF, "dhcpd.conf") or die "Cannot read from file: $!"; open (DHCPDCONFNEW, ">dhcpd.conf.new") or die "Cannot create file: $!" +; while( <DHCPDCONF>) { my $device = lc($1) if /#.#.#.# Modem MAC: (\w+:\w+:\w+:\w+:\w+ +:\w+)/; s/(#.#.# Modem IP: ).*( # Modem MAC: $device)/$1$devices{$device +}$2/i; print DHCPDCONFNEW; } close DHCPDCONF; close DHCPDCONFNEW; system("cp dhcpd.conf.new dhcpd.conf"); ######## Update LDAP database # not implemented yet. [download]	[reply] [d/l] [select]
Re: Re: Re: script hangs up when reading in file by Gilimanjaro (Hermit) on Jan 20, 2003 at 11:35 UTC
Looks good! The reason the old code seemed to hang is probably because of the way the for loop made things happen; The 'keys' at the loop initializer extracted 2300+ keys from a hash. That's hefty operation 1. Then 2300+ times, you extract a single entry from the hash with the key you named (hefty op 2), and perform a substitution regex with match variabels like $1/$2 (hefty op 3). And you store it in a stringus humangeous. (Hefty op 4). Now you're doing it nice and and gentle, one line at a time... :) About the while instead of foreach; the 'each' operator is a very gentle way to handle hashes, because it just traverses the hash instead of performing an operation which involves the entire thing. And you can use it to get the key and value in one swoop. It might not look as 'english' as the foreach construct, but it's worth it! Anyway, glad I could help... Happy coding!	[reply]