From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 4390F1F47A for ; Wed, 25 Dec 2019 07:51:05 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 03/30] qspawn: remove some anonymous subs for psgi_qx Date: Wed, 25 Dec 2019 07:50:37 +0000 Message-Id: <20191225075104.22184-4-e@80x24.org> In-Reply-To: <20191225075104.22184-1-e@80x24.org> References: <20191225075104.22184-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: By passing a user-supplied arg to $qx_cb, we can eliminate the callers' need to capture on-stack variables with a closure. This saves several kilobytes of memory allocation at the expense of some extra hash table lookups in user-supplied callbacks. It also reduces the risk of memory leaks by eliminating a common source of circular references. --- lib/PublicInbox/Qspawn.pm | 4 +- lib/PublicInbox/SolverGit.pm | 88 ++++++++++++++++++++---------------- lib/PublicInbox/ViewVCS.pm | 33 ++++++++------ 3 files changed, 71 insertions(+), 54 deletions(-) diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm index c2856609..22603ca7 100644 --- a/lib/PublicInbox/Qspawn.pm +++ b/lib/PublicInbox/Qspawn.pm @@ -155,13 +155,13 @@ sub start { # $env is the PSGI env. As with ``/qx; only use this when output is small # and safe to slurp. sub psgi_qx { - my ($self, $env, $limiter, $qx_cb) = @_; + my ($self, $env, $limiter, $qx_cb, $cb_arg) = @_; my $scalar = ''; open(my $qx, '+>', \$scalar) or die; # PerlIO::scalar my $end = sub { my $err = $_[0]; # $! log_err($env, "psgi_qx: $err") if defined($err); - finish($self, $env, sub { $qx_cb->(\$scalar) }); + finish($self, $env, sub { $qx_cb->(\$scalar, $cb_arg) }); $qx = undef; }; my $rpipe; # comes from popen_rd diff --git a/lib/PublicInbox/SolverGit.pm b/lib/PublicInbox/SolverGit.pm index d59320bc..f81f69ca 100644 --- a/lib/PublicInbox/SolverGit.pm +++ b/lib/PublicInbox/SolverGit.pm @@ -221,6 +221,16 @@ sub find_extract_diffs ($$$) { @di ? \@di : undef; } +sub update_index_result ($$) { + my ($bref, $self) = @_; + my ($qsp, $msg) = delete @$self{qw(-qsp -msg)}; + if (my $err = $qsp->{err}) { + ERR($self, "git update-index error: $err"); + } + dbg($self, $msg); + next_step($self); # onto do_git_apply +} + sub prepare_index ($) { my ($self) = @_; my $patches = $self->{patches}; @@ -248,15 +258,10 @@ sub prepare_index ($) { my $rdr = { 0 => fileno($in), -hold => $in }; my $cmd = [ qw(git update-index -z --index-info) ]; my $qsp = PublicInbox::Qspawn->new($cmd, $self->{git_env}, $rdr); - $qsp->psgi_qx($self->{psgi_env}, undef, sub { - my ($bref) = @_; - if (my $err = $qsp->{err}) { - ERR($self, "git update-index error: $err"); - } - dbg($self, "index prepared:\n" . - "$mode_a $oid_full\t" . git_quote($path_a)); - next_step($self); # onto do_git_apply - }); + $path_a = git_quote($path_a); + $self->{-qsp} = $qsp; + $self->{-msg} = "index prepared:\n$mode_a $oid_full\t$path_a"; + $qsp->psgi_qx($self->{psgi_env}, undef, \&update_index_result, $self); } # pure Perl "git init" @@ -383,8 +388,9 @@ sub mark_found ($$$) { } } -sub parse_ls_files ($$$$) { - my ($self, $qsp, $bref, $di) = @_; +sub parse_ls_files ($$) { + my ($self, $bref) = @_; + my ($qsp, $di) = delete @$self{qw(-qsp -cur_di)}; if (my $err = $qsp->{err}) { die "git ls-files error: $err"; } @@ -410,15 +416,10 @@ sub parse_ls_files ($$$$) { next_step($self); # onto the next patch } -sub start_ls_files ($$) { - my ($self, $di) = @_; - my $cmd = [qw(git ls-files -s -z)]; - my $qsp = PublicInbox::Qspawn->new($cmd, $self->{git_env}); - $qsp->psgi_qx($self->{psgi_env}, undef, sub { - my ($bref) = @_; - eval { parse_ls_files($self, $qsp, $bref, $di) }; - ERR($self, $@) if $@; - }); +sub ls_files_result { + my ($bref, $self) = @_; + eval { parse_ls_files($self, $bref) }; + ERR($self, $@) if $@; } sub oids_same_ish ($$) { @@ -438,6 +439,31 @@ sub skip_identical ($$$) { } } +sub apply_result ($$) { + my ($bref, $self) = @_; + my ($qsp, $di) = delete @$self{qw(-qsp -cur_di)}; + dbg($self, $$bref); + my $patches = $self->{patches}; + if (my $err = $qsp->{err}) { + my $msg = "git apply error: $err"; + my $nxt = $patches->[0]; + if ($nxt && oids_same_ish($nxt->{oid_b}, $di->{oid_b})) { + dbg($self, $msg); + dbg($self, 'trying '.di_url($self, $nxt)); + } else { + ERR($self, $msg); + } + } else { + skip_identical($self, $patches, $di->{oid_b}); + } + + my @cmd = qw(git ls-files -s -z); + $qsp = PublicInbox::Qspawn->new(\@cmd, $self->{git_env}); + $self->{-cur_di} = $di; + $self->{-qsp} = $qsp; + $qsp->psgi_qx($self->{psgi_env}, undef, \&ls_files_result, $self); +} + sub do_git_apply ($) { my ($self) = @_; my $dn = $self->{tmp}->dirname; @@ -465,24 +491,9 @@ sub do_git_apply ($) { my $rdr = { 2 => 1 }; my $qsp = PublicInbox::Qspawn->new(\@cmd, $self->{git_env}, $rdr); - $qsp->psgi_qx($self->{psgi_env}, undef, sub { - my ($bref) = @_; - dbg($self, $$bref); - if (my $err = $qsp->{err}) { - my $msg = "git apply error: $err"; - my $nxt = $patches->[0]; - if ($nxt && oids_same_ish($nxt->{oid_b}, $prv_oid_b)) { - dbg($self, $msg); - dbg($self, 'trying '.di_url($self, $nxt)); - } else { - ERR($self, $msg); - } - } else { - skip_identical($self, $patches, $di->{oid_b}); - } - eval { start_ls_files($self, $di) }; - ERR($self, $@) if $@; - }); + $self->{-cur_di} = $di; + $self->{-qsp} = $qsp; + $qsp->psgi_qx($self->{psgi_env}, undef, \&apply_result, $self); } sub di_url ($$) { @@ -564,6 +575,7 @@ sub new { bless { gits => $ibx->{-repo_objs}, user_cb => $user_cb, + # -cur_di, -qsp, -msg => temporary fields for Qspawn callbacks # TODO: config option for searching related inboxes inboxes => [ $ibx ], diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm index 842c873c..886e10cb 100644 --- a/lib/PublicInbox/ViewVCS.pm +++ b/lib/PublicInbox/ViewVCS.pm @@ -73,6 +73,22 @@ sub stream_large_blob ($$$$) { }); } +sub show_other_result ($$) { + my ($bref, $ctx) = @_; + my ($qsp, $logref) = delete @$ctx{qw(-qsp -logref)}; + if (my $err = $qsp->{err}) { + utf8::decode($$err); + $$logref .= "git show error: $err"; + return html_page($ctx, 500, $logref); + } + my $l = PublicInbox::Linkify->new; + utf8::decode($$bref); + $l->linkify_1($$bref); + $$bref = '
'. $l->linkify_2(ascii_html($$bref));
+	$$bref .= '
' . $$logref; + html_page($ctx, 200, $bref); +} + sub show_other ($$$$) { my ($ctx, $res, $logref, $fn) = @_; my ($git, $oid, $type, $size) = @$res; @@ -84,20 +100,9 @@ sub show_other ($$$$) { qw(show --encoding=UTF-8 --no-color --no-abbrev), $oid ]; my $qsp = PublicInbox::Qspawn->new($cmd); my $env = $ctx->{env}; - $qsp->psgi_qx($env, undef, sub { - my ($bref) = @_; - if (my $err = $qsp->{err}) { - utf8::decode($$err); - $$logref .= "git show error: $err"; - return html_page($ctx, 500, $logref); - } - my $l = PublicInbox::Linkify->new; - utf8::decode($$bref); - $l->linkify_1($$bref); - $$bref = '
'. $l->linkify_2(ascii_html($$bref));
-		$$bref .= '
' . $$logref; - html_page($ctx, 200, $bref); - }); + $ctx->{-qsp} = $qsp; + $ctx->{-logref} = $logref; + $qsp->psgi_qx($env, undef, \&show_other_result, $ctx); } sub solve_result {