]> Sergey Matveev's repositories - public-inbox.git/commitdiff
treewide: avoid `goto &NAME' for tail recursion
authorEric Wong <e@80x24.org>
Fri, 11 Sep 2020 07:32:31 +0000 (07:32 +0000)
committerEric Wong <e@80x24.org>
Sat, 12 Sep 2020 20:29:11 +0000 (20:29 +0000)
While Perl implements tail recursion via `goto' which allows
avoiding warnings on deep recursion.  It doesn't (as of 5.28)
optimize the speed of such dispatches, though it may reduce
ephemeral memory usage.

Make the code less alien to hackers coming from other languages
by using normal subroutine dispatch.  It's actually slightly
faster in micro benchmarks due to the complexity of `goto &NAME'.

lib/PublicInbox/DS.pm
lib/PublicInbox/Eml.pm
lib/PublicInbox/ExtMsg.pm
lib/PublicInbox/SolverGit.pm
lib/PublicInbox/V2Writable.pm
lib/PublicInbox/View.pm
lib/PublicInbox/Watch.pm

index 661be1fd2c4553723e07587f26e04be4adb5ffb1..9c2783073d047928693c25b30f22b75b0e066a56 100644 (file)
@@ -244,7 +244,7 @@ sub reap_pids {
 }
 
 # reentrant SIGCHLD handler (since reap_pids is not reentrant)
-sub enqueue_reap { $reap_armed //= requeue(\&reap_pids) }
+sub enqueue_reap () { $reap_armed //= requeue(\&reap_pids) }
 
 sub in_loop () { $in_loop }
 
@@ -629,7 +629,7 @@ sub dwaitpid ($$$) {
        push @$wait_pids, [ @_ ]; # [ $pid, $cb, $arg ]
 
        # We could've just missed our SIGCHLD, cover it, here:
-       goto &enqueue_reap; # tail recursion
+       enqueue_reap();
 }
 
 sub _run_later () {
index 1fecd41be9f4b1f2d11d4591918935ac9b1aba16..571edc5c950c7ccd575e6de91ce1326f446ab788 100644 (file)
@@ -129,7 +129,7 @@ sub new {
 sub new_sub {
        my (undef, $ref) = @_;
        # special case for messages like <85k5su9k59.fsf_-_@lola.goethe.zz>
-       $$ref =~ /\A(\r?\n)/s or goto &new;
+       $$ref =~ /\A(\r?\n)/s or return new(undef, $ref);
        my $hdr = substr($$ref, 0, $+[0], ''); # sv_chop on $$ref
        bless { hdr => \$hdr, crlf => $1, bdy => $ref }, __PACKAGE__;
 }
index ce1a47bb84fd70da06e617dd0d1ee34a2e409b99..03faf3a142cdc46d65fc4287edcf01191ac4182a 100644 (file)
@@ -125,12 +125,12 @@ sub ext_msg {
 sub event_step {
        my ($ctx, $sync) = @_;
        # can't find a partial match in current inbox, try the others:
-       my $ibx = shift @{$ctx->{again}} or goto \&finalize_partial;
+       my $ibx = shift @{$ctx->{again}} or return finalize_partial($ctx);
        my $mids = search_partial($ibx, $ctx->{mid}) or
                        return ($sync ? undef : PublicInbox::DS::requeue($ctx));
        $ctx->{n_partial} += scalar(@$mids);
        push @{$ctx->{partial}}, [ $ibx, $mids ];
-       $ctx->{n_partial} >= PARTIAL_MAX ? goto(\&finalize_partial)
+       $ctx->{n_partial} >= PARTIAL_MAX ? finalize_partial($ctx)
                        : ($sync ? undef : PublicInbox::DS::requeue($ctx));
 }
 
@@ -156,7 +156,7 @@ sub finalize_exact {
                # synchronous fall-through
                $ctx->event_step while @{$ctx->{again}};
        }
-       goto \&finalize_partial;
+       finalize_partial($ctx);
 }
 
 sub finalize_partial {
index ae3997ca6d06e238176ec797f42965771dc444b6..83f7a4eefc6a5cc6f78d0cc9af48aaab9656694f 100644 (file)
@@ -517,16 +517,16 @@ sub di_url ($$) {
 }
 
 sub retry_current {
-       my ($self, $want) = @_;
-       push @{$_[0]->{todo}}, $_[1];
-       goto \&next_step # retry solve_existing
+       my ($self, $want) = @_;
+       push @{$self->{todo}}, $want;
+       next_step($self); # retry solve_existing
 }
 
-sub try_harder {
+sub try_harder ($$) {
        my ($self, $want) = @_;
 
        # do we have more inboxes to try?
-       goto \&retry_current if scalar @{$want->{try_ibxs}};
+       return retry_current($self, $want) if scalar @{$want->{try_ibxs}};
 
        my $cur_want = $want->{oid_b};
        if (length($cur_want) > $OID_MIN) { # maybe a shorter OID will work
@@ -534,7 +534,7 @@ sub try_harder {
                chop($cur_want);
                dbg($self, "retrying $want->{oid_b} as $cur_want");
                $want->{oid_b} = $cur_want;
-               goto \&retry_current; # retry with shorter abbrev
+               return retry_current($self, $want); # retry with shorter abbrev
        }
 
        dbg($self, "could not find $cur_want");
@@ -564,9 +564,9 @@ sub extract_diffs_done {
                        my $job = { oid_b => $src, path_b => $di->{path_a} };
                        push @{$self->{todo}}, $job;
                }
-               goto \&next_step; # onto the next todo item
+               return next_step($self); # onto the next todo item
        }
-       goto \&try_harder;
+       try_harder($self, $want);
 }
 
 sub extract_diff_async {
@@ -578,9 +578,8 @@ sub extract_diff_async {
                PublicInbox::Eml->new($bref)->each_part(\&extract_diff, $x, 1);
        }
 
-       scalar(@{$want->{try_smsgs}}) ?
-               retry_current($self, $want) :
-               extract_diffs_done($self, $want);
+       scalar(@{$want->{try_smsgs}}) ? retry_current($self, $want)
+                                       : extract_diffs_done($self, $want);
 }
 
 sub resolve_patch ($$) {
@@ -605,7 +604,8 @@ sub resolve_patch ($$) {
                        }
                }
 
-               goto(scalar @$msgs ? \&retry_current : \&extract_diffs_done);
+               return scalar(@$msgs) ? retry_current($self, $want)
+                                       : extract_diffs_done($self, $want);
        }
 
        # see if we can find the blob in an existing git repo:
@@ -626,9 +626,9 @@ sub resolve_patch ($$) {
                        return;
                }
                mark_found($self, $cur_want, $existing);
-               goto \&next_step; # onto patch application
+               return next_step($self); # onto patch application
        } elsif ($existing > 0) {
-               goto \&retry_current;
+               return retry_current($self, $want);
        } else { # $existing == 0: we may retry if inbox scan (below) fails
                delete $want->{try_gits};
        }
@@ -640,9 +640,9 @@ sub resolve_patch ($$) {
                $want->{try_smsgs} = $msgs;
                $want->{cur_ibx} = $ibx;
                $self->{tmp_diffs} = [];
-               goto \&retry_current;
+               return retry_current($self, $want);
        }
-       goto \&try_harder;
+       try_harder($self, $want);
 }
 
 # this API is designed to avoid creating self-referential structures;
index a1f6048f3b27113b697baa1c48c41c8e4334bc10..b8abfa94bdebb97a7cc48d2fc82465d035777f21 100644 (file)
@@ -1267,8 +1267,8 @@ sub xapian_only {
 # public, called by public-inbox-index
 sub index_sync {
        my ($self, $opt) = @_;
-       $opt //= $_[1] //= {};
-       goto \&xapian_only if $opt->{xapian_only};
+       $opt //= {};
+       return xapian_only($self, $opt) if $opt->{xapian_only};
 
        my $pr = $opt->{-progress};
        my $epoch_max;
index 3055da20890260d473f11d5ca9c6c0800cb75af7..1d5119cd1bdf9a451e91ba5fd8a9bc13aec9a957 100644 (file)
@@ -386,7 +386,7 @@ sub next_in_queue ($$) {
 
 sub stream_thread_i { # PublicInbox::WwwStream::getline callback
        my ($ctx, $eml) = @_;
-       goto &thread_eml_entry if $eml; # tail recursion
+       return thread_eml_entry($ctx, $eml) if $eml;
        return unless exists($ctx->{skel});
        my $ghost_ok = $ctx->{nr}++;
        while (1) {
index 0f41dff289d6e72a318fc85e7c2dafbcf023f3bd..8bbce92992aa004e26271394bb0c75cffa31fb5d 100644 (file)
@@ -651,7 +651,7 @@ sub event_step {
                PublicInbox::Sigfd::sig_setmask($oldset);
                die $@ if $@;
        }
-       goto(&fs_scan_step) if $self->{mdre};
+       fs_scan_step($self) if $self->{mdre};
 }
 
 sub watch_imap_fetch_all ($$) {
@@ -1066,7 +1066,7 @@ sub fs_scan_step {
 sub scan {
        my ($self, $op) = @_;
        push @{$self->{ops}}, $op;
-       goto &fs_scan_step;
+       fs_scan_step($self);
 }
 
 sub _importer_for {