about summary refs log tree commit
diff options
context:
space:
mode:
authorEric Wong <e@80x24.org>2019-09-17 08:31:20 +0000
committerEric Wong <e@80x24.org>2019-09-17 08:31:44 +0000
commit685da5f572042faa54fb4479c222d4f2d258e2ed (patch)
treebab369d4db5a7489d543a64905088a8d416502aa
parent641191aa902bb3259a13d0f348b78a49aafe4902 (diff)
downloadpublic-inbox-685da5f572042faa54fb4479c222d4f2d258e2ed.tar.gz
All of these circular references are designed to clear
themselves, but these will make actual errors from Devel::Cycle
easier-to-spot.

The circular reference in the limiter {run_queue} is not a real
problem, but we can avoid storing the circular reference until
we actually need to spawn the child, reducing the size of the
Qspawn object while it's in the queue, slightly.

We also do not need to have redundant checks to spawn new
processes, we should only spawn new processes when they're
->start-ed or after waitpid reaps them.
-rw-r--r--lib/PublicInbox/Qspawn.pm28
1 files changed, 9 insertions, 19 deletions
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 844d50f7..10fe5341 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -44,11 +44,11 @@ sub new ($$$;) {
 }
 
 sub _do_spawn {
-        my ($self, $cb) = @_;
+        my ($self, $cb, $limiter) = @_;
         my $err;
         my ($cmd, $env, $opts) = @{$self->{args}};
         my %opts = %{$opts || {}};
-        my $limiter = $self->{limiter};
+        $self->{limiter} = $limiter;
         foreach my $k (PublicInbox::Spawn::RLIMITS()) {
                 if (defined(my $rlimit = $limiter->{$k})) {
                         $opts{$k} = $rlimit;
@@ -94,21 +94,21 @@ sub waitpid_err ($$) {
                 $err = "W: waitpid($xpid, 0) => $pid: $!";
         } # else should not be called with pid == 0
 
+        my $env = delete $self->{env};
+
         # done, spawn whatever's in the queue
         my $limiter = $self->{limiter};
         my $running = --$limiter->{running};
 
-        # limiter->{max} may change dynamically
-        if (($running || $limiter->{running}) < $limiter->{max}) {
-                if (my $next = shift @{$limiter->{run_queue}}) {
-                        _do_spawn(@$next);
+        if ($running < $limiter->{max}) {
+                if (my $next = shift(@{$limiter->{run_queue}})) {
+                        _do_spawn(@$next, $limiter);
                 }
         }
 
         return unless $err;
         $self->{err} = $err;
-        my $env = $self->{env} or return;
-        if (!$env->{'qspawn.quiet'}) {
+        if ($env && !$env->{'qspawn.quiet'}) {
                 log_err($env, join(' ', @{$self->{args}}) . ": $err");
         }
 }
@@ -132,22 +132,12 @@ sub finish ($;$) {
         if (delete $self->{rpipe}) {
                 do_waitpid($self, $env);
         }
-
-        # limiter->{max} may change dynamically
-        my $limiter = $self->{limiter};
-        if ($limiter->{running} < $limiter->{max}) {
-                if (my $next = shift @{$limiter->{run_queue}}) {
-                        _do_spawn(@$next);
-                }
-        }
 }
 
 sub start {
         my ($self, $limiter, $cb) = @_;
-        $self->{limiter} = $limiter;
-
         if ($limiter->{running} < $limiter->{max}) {
-                _do_spawn($self, $cb);
+                _do_spawn($self, $cb, $limiter);
         } else {
                 push @{$limiter->{run_queue}}, [ $self, $cb ];
         }