On Thu, Jul 26, 2012 at 11:01 AM, Shlomi Fish <shlo...@shlomifish.org>wrote:
> Hi Punit,
>
> a few comments on your code.
>
> On Thu, 26 Jul 2012 10:17:13 +0530
> punit jain <contactpunitj...@gmail.com> wrote:
>
> > Hi,
> >
> > Below is my script where alarm is generated after 15 sec and changes the
> > global variable in parent from 0 to 1. Is there any mistake in my
> > multiprocess approach with time-based stop ?
> >
> > #!/usr/bin/perl
> > use strict;
> > use warnings;
> > use Parallel::ForkManager;
>
> It's good that you're using "strict" and "warnings", but you lack more
> empty
> lines between paragraphs in your code.
>
> >
> > my $MAX_PROC=2;
> > $|++;
> > my $stopFlag=0;
>
> You should use IO::Handle->autoflush(1) instead of $|++ for clarity.
>
> >
> > main();
> >
> > sub main {
> >
> > preprocess(@ARGV);
> > multiproc($MAX_PROC, @ARGV);
> >
> > }
>
> Your indentation is bad. Where is preprocess() defined? Moreover, I should
> note
> that passing @ARGV directly to subroutine smells of lack of separation of
> concerns.
>
> > sub multiproc {
> >
> > my $proc=shift;
>
> Your indentation is bad and you need spaces surrounding the "=".
>
> > my $argu = $ARGV[0];
>
> Don't use $ARGV[0] directly:
>
> http://perl-begin.org/tutorials/bad-elements/#subroutine-arguments
>
> >
> > open(USRFILE, "$argu") or die "cant open $!";
>
> 1. Don't use bareword file handles.
>
> 2. Use three-args opens.
>
> 3. Don't surround strings with quotations.
>
> See:
>
> * http://perl-begin.org/tutorials/bad-elements/#vars_in_quotes
>
> * http://perl-begin.org/tutorials/bad-elements/#open-function-style
>
> You also should use a more descriptive and cohernet name than USRFILE and
> also
> see:
>
> http://perl-begin.org/tutorials/bad-elements/#calling-variables-file
>
> > my $pmgr = Parallel::ForkManager->new($
> > proc);
> >
> > $pmgr->run_on_finish(
> > sub { my ($pid, $exit_code, $ident) = @_;
>
> Don't put the argument unpacking on the same line as the "{".
>
> > print "** $ident just got out of the pool ** with PID $pid and
> > parent pid as $$ exit code: $exit_code\n";
> > }
> > );
> >
> > $pmgr-> run_on_start(
> > sub { my ($pid,$ident)=@_;
>
> Again, and you have a space after the "->".
>
> > print "** $ident started, pid: $pid **\n";
> > }
> > );
> >
> > $SIG{ALRM} = sub { $stopFlag = 1; };
> > alarm(16);
> >
> > while(my $user = <USRFILE>) {
> > chomp($user);
> > print "value of Stop Flag in parent $0 is $stopFlag\n";
> > if($stopFlag == 1) {
> > print "stop flag has been set\n";
> > last;
> > }
>
> Just use if ($stopFlag) here.
>
> > my $id = $pmgr->start($user) and next;
> > print "value of Stop Flag in child $id is $stopFlag\n";
> > sleep(7);
> >
> > $pmgr->finish($user);
> > }
> > print "Waiting for all children to exit\n";
>
> Again - bad indentation.
>
> Regards,
>
> Shlomi Fish
>
> > $pmgr->wait_all_children();
> > alarm(0);
> > print "All children completed\n";
> > }
> >
> >
> > This is what I get output : -
> >
> > perl mprocess.pl /tmp/list1
> > value of Stop Flag in parent mprocess.pl is 0
> > value of Stop Flag in child 0 is 0
> > ** te...@test.com started, pid: 21777 **
> > value of Stop Flag in parent mprocess.pl is 0
> > value of Stop Flag in child 0 is 0
> > ** te...@test.com started, pid: 21778 **
> > value of Stop Flag in parent mprocess.pl is 0
> > ** te...@test.com just got out of the pool ** with PID 21777 and parent
> pid
> > as 21776 exit code: 0
> > ** te...@test.com just got out of the pool ** with PID 21778 and parent
> pid
> > as 21776 exit code: 0
> > value of Stop Flag in child 0 is 0
> > ** te...@test.com started, pid: 21811 **
> > value of Stop Flag in parent mprocess.pl is 0
> > value of Stop Flag in child 0 is 0
> > ** te...@test.com started, pid: 21812 **
> > value of Stop Flag in parent mprocess.pl is 0
> > ** te...@test.com just got out of the pool ** with PID 21811 and parent
> pid
> > as 21776 exit code: 0
> > ** te...@test.com just got out of the pool ** with PID 21812 and parent
> pid
> > as 21776 exit code: 0
> > value of Stop Flag in child 0 is 0
> > ** te...@test.com started, pid: 21832 **
> > value of Stop Flag in parent mprocess.pl is 0
> > value of Stop Flag in child 0 is 0
> > ** te...@test.com started, pid: 21833 **
> > value of Stop Flag in parent mprocess.pl is 0
> > ** te...@test.com just got out of the pool ** with PID 21832 and parent
> pid
> > as 21776 exit code: 0
> > ** te...@test.com just got out of the pool ** with PID 21833 and parent
> pid
> > as 21776 exit code: 0
> > value of Stop Flag in child 0 is 1
> > ** te...@test.com started, pid: 22030 **
> > value of Stop Flag in parent mprocess.pl is 1
> > stop flag has been set
> > Waiting for all children to exit
> > ** te...@test.com just got out of the pool ** with PID 22030 and parent
> pid
> > as 21776 exit code: 0
> > All children completed
> >
> > The concern here is I see value of stopflag as 1 for child before the
> > parent which is a bit wierd:-
> >
> > value of Stop Flag in child 0 is 1
> > ** te...@test.com started, pid: 22030 **
> > value of Stop Flag in parent mprocess.pl is 1
> >
> > Thanks and Regards.
>
>
>
> --
> -----------------------------------------------------------------
> Shlomi Fish http://www.shlomifish.org/
> List of Portability Libraries - http://shlom.in/port-libs
>
> Jewish Atheists are the only true Atheists. They beat the hell out of Goy
> Atheists.
>
> Please reply to list if it's a mailing list post - http://shlom.in/reply .
>
> --
> To unsubscribe, e-mail: beginners-unsubscr...@perl.org
> For additional commands, e-mail: beginners-h...@perl.org
> http://learn.perl.org/
>
>
>
Though I cannot fault Shlomi on his style advice I don't think this is what
you where looking for :-)
Shlomi is quite right to point out the fact that your code looks like it
was done in a hurry and is certainly not production quality and could do
with some improvements. The good thing is that Shlomi is besides being a
stickler for style and good code also a quite good perl coder and I know
that if he did find fault in your code he would have pointed it out.
As for me I tend to vary in style and code cleanliness myself depending on
the goal and the iteration. :-) Your main question is if the code you wrote
is done well or if it could be done better. Personally I can't see anything
terribly wrong with it except for the style and total lack of comments
(these are pointless for you but the next guy that comes along and looks at
your code will be thankful for the few moments you spend on that).
Your other question about the order in which the variables change this is
not to strange, it might actually not be the order in which the variables
change. The only thing you see is the order in which the output is pushed
to the buffer it might very well be that process A simply didn't yield yet
and managed to push its string to the buffer before the other process got
the chance to do so.
As long as the code works correctly and the variables change as expected
(maybe not in the right order in the logs) you should not worry to much
about the order in which this shows up in the logs.
Rob.
