From 65db62eb006fdbe74348f0e8f2ef9b9e938cb90b Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 26 Jan 2023 09:32:56 +0000 Subject: git: use --batch-command in git 2.36+ to save processes `git cat-file --batch-command' combines the functionality of `--batch' and `--batch-check' into a single process. This reduces the amount of running processes and is primarily useful for coderepos (e.g. solver). This also fixes prior use of `print { $git->{out} }' which is a a potential (but unlikely) bug since commit d4ba8828ab23f278 (git: fix asynchronous batching for deep pipelines, 2023-01-04) Lack of libgit2 on one of my test machines also uncovered fixes necessary for t/imapd.t, t/nntpd.t and t/nntpd-v2.t. --- lib/PublicInbox/Git.pm | 152 ++++++++++++++++++++++++++++------------- lib/PublicInbox/GitAsyncCat.pm | 9 +-- t/nntpd.t | 2 +- 3 files changed, 108 insertions(+), 55 deletions(-) diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index 12f997dc..70adfd45 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -31,7 +31,10 @@ our $async_warn; # true in read-only daemons # 512: POSIX PIPE_BUF minimum (see pipe(7)) # 3: @$inflight is flattened [ $OID, $cb, $arg ] # 65: SHA-256 hex size + "\n" in preparation for git using non-SHA1 -use constant MAX_INFLIGHT => 512 * 3 / 65; +use constant { + MAX_INFLIGHT => 512 * 3 / (65 + length('contents ')), + BATCH_CMD_VER => (2 << 24 | 36 << 16), # git 2.36+ +}; my %GIT_ESC = ( a => "\a", @@ -46,6 +49,24 @@ my %GIT_ESC = ( ); my %ESC_GIT = map { $GIT_ESC{$_} => $_ } keys %GIT_ESC; +my $EXE_ST = ''; # pack('dd', st_ctime, st_size); +my ($GIT_EXE, $GIT_VER); + +sub check_git_exe () { + $GIT_EXE = which('git') // die "git not found in $ENV{PATH}"; + my @st = stat($GIT_EXE) or die "stat: $!"; + my $st = pack('dd', $st[10], $st[7]); + if ($st ne $EXE_ST) { + my $rd = popen_rd([ $GIT_EXE, '--version' ]); + my $v = readline($rd); + $v =~ /\b([0-9]+(?:\.[0-9]+){2})/ or die + "$GIT_EXE --version output: $v # unparseable"; + my @v = split(/\./, $1, 3); + $GIT_VER = ($v[0] << 24) | ($v[1] << 16) | $v[2]; + $EXE_ST = $st; + } +} + # unquote pathnames used by git, see quote.c::unquote_c_style.c in git.git sub git_unquote ($) { return $_[0] unless ($_[0] =~ /\A"(.*)"\z/); @@ -122,25 +143,6 @@ sub _bidi_pipe { return; } - state $EXE_ST = ''; # pack('dd', st_ctime, st_size); - my $exe = which('git') // die "git not found in $ENV{PATH}"; - my @st = stat($exe) or die "stat: $!"; - my $st = pack('dd', $st[10], $st[7]); - state $VER; - if ($st ne $EXE_ST) { - my $rd = popen_rd([ $exe, '--version' ]); - my $v = readline($rd); - $v =~ /\b([0-9]+(?:\.[0-9]+){2})/ or die - "$exe --version output: $v # unparseable"; - my @v = split(/\./, $1, 3); - $VER = ($v[0] << 24) | ($v[1] << 16) | $v[2]; - $EXE_ST = $st; - } - - # git 2.31.0+ supports -c core.abbrev=no, don't bother with - # core.abbrev=64 since not many releases had SHA-256 prior to 2.31 - my $abbr = $VER < (2 << 24 | 31 << 16) ? 40 : 'no'; - pipe(my ($out_r, $out_w)) or $self->fail("pipe failed: $!"); my $rdr = { 0 => $out_r, pgid => 0 }; my $gd = $self->{git_dir}; @@ -148,10 +150,14 @@ sub _bidi_pipe { $rdr->{-C} = $gd; $gd = $1; } - my @cmd = ($exe, "--git-dir=$gd", '-c', "core.abbrev=$abbr", - 'cat-file', $batch); + + # git 2.31.0+ supports -c core.abbrev=no, don't bother with + # core.abbrev=64 since not many releases had SHA-256 prior to 2.31 + my $abbr = $GIT_VER < (2 << 24 | 31 << 16) ? 40 : 'no'; + my @cmd = ($GIT_EXE, "--git-dir=$gd", '-c', "core.abbrev=$abbr", + 'cat-file', "--$batch"); if ($err) { - my $id = "git.$self->{git_dir}$batch.err"; + my $id = "git.$self->{git_dir}.$batch.err"; my $fh = tmpfile($id) or $self->fail("tmpfile($id): $!"); $self->{$err} = $fh; $rdr->{2} = $fh; @@ -163,7 +169,7 @@ sub _bidi_pipe { $out_w->autoflush(1); if ($^O eq 'linux') { # 1031: F_SETPIPE_SZ fcntl($out_w, 1031, 4096); - fcntl($in_r, 1031, 4096) if $batch eq '--batch-check'; + fcntl($in_r, 1031, 4096) if $batch eq 'batch-check'; } $out_w->blocking(0); $self->{$out} = $out_w; @@ -233,6 +239,16 @@ sub cat_async_retry ($$) { cat_async_step($self, $inflight); # take one step } +# returns true if prefetch is successful +sub async_prefetch { + my ($self, $oid, $cb, $arg) = @_; + my $inflight = $self->{inflight} or return; + return if @$inflight; + substr($oid, 0, 0) = 'contents ' if $self->{-bc}; + write_all($self, $self->{out}, "$oid\n", \&cat_async_step, $inflight); + push(@$inflight, $oid, $cb, $arg); +} + sub cat_async_step ($$) { my ($self, $inflight) = @_; die 'BUG: inflight empty or odd' if scalar(@$inflight) < 3; @@ -240,12 +256,22 @@ sub cat_async_step ($$) { my $rbuf = delete($self->{rbuf}) // \(my $new = ''); my ($bref, $oid, $type, $size); my $head = my_readline($self->{in}, $rbuf); + my $cmd = ref($req) ? $$req : $req; # ->fail may be called via Gcf2Client.pm + my $bc = $self->{-bc}; if ($head =~ /^([0-9a-f]{40,}) (\S+) ([0-9]+)$/) { ($oid, $type, $size) = ($1, $2, $3 + 0); - $bref = my_read($self->{in}, $rbuf, $size + 1) or - $self->fail(defined($bref) ? 'read EOF' : "read: $!"); - chop($$bref) eq "\n" or $self->fail('LF missing after blob'); + unless ($bc && $cmd =~ /\Ainfo /) { # --batch-command + $bref = my_read($self->{in}, $rbuf, $size + 1) or + $self->fail(defined($bref) ? + 'read EOF' : "read: $!"); + chop($$bref) eq "\n" or + $self->fail('LF missing after blob'); + } + } elsif ($bc && $cmd =~ /\Ainfo / && + $head =~ / (missing|ambiguous)\n/) { + $type = $1; + $oid = substr($cmd, 5); } elsif ($head =~ s/ missing\n//s) { $oid = $head; # ref($req) indicates it's already been retried @@ -254,15 +280,23 @@ sub cat_async_step ($$) { return cat_async_retry($self, $inflight); } $type = 'missing'; - $oid = ref($req) ? $$req : $req if $oid eq ''; + if ($oid eq '') { + $oid = $cmd; + $oid =~ s/\A(?:contents|info) // if $bc; + } } else { my $err = $! ? " ($!)" : ''; $self->fail("bad result from async cat-file: $head$err"); } $self->{rbuf} = $rbuf if $$rbuf ne ''; splice(@$inflight, 0, 3); # don't retry $cb on ->fail - eval { $cb->($bref, $oid, $type, $size, $arg) }; - async_err($self, $req, $oid, $@, 'cat') if $@; + if ($bc && $cmd =~ /\Ainfo /) { + eval { $cb->($oid, $type, $size, $arg, $self) }; + async_err($self, $req, $oid, $@, 'check') if $@; + } else { + eval { $cb->($bref, $oid, $type, $size, $arg) }; + async_err($self, $req, $oid, $@, 'cat') if $@; + } } sub cat_async_wait ($) { @@ -274,7 +308,15 @@ sub cat_async_wait ($) { } sub batch_prepare ($) { - _bidi_pipe($_[0], qw(--batch in out pid)); + my ($self) = @_; + check_git_exe(); + if ($GIT_VER >= BATCH_CMD_VER) { + _bidi_pipe($self, qw(batch-command in out pid err_c)); + $self->{-bc} = 1; + } else { + _bidi_pipe($self, qw(batch in out pid)); + } + $self->{inflight} = []; } sub _cat_file_cb { @@ -314,18 +356,24 @@ sub check_async_step ($$) { sub check_async_wait ($) { my ($self) = @_; + return cat_async_wait($self) if $self->{-bc}; my $inflight_c = $self->{inflight_c} or return; - while (scalar(@$inflight_c)) { - check_async_step($self, $inflight_c); - } + check_async_step($self, $inflight_c) while (scalar(@$inflight_c)); } sub check_async_begin ($) { my ($self) = @_; - cleanup($self) if alternates_changed($self); - _bidi_pipe($self, qw(--batch-check in_c out_c pid_c err_c)); die 'BUG: already in async check' if $self->{inflight_c}; - $self->{inflight_c} = []; + cleanup($self) if alternates_changed($self); + check_git_exe(); + if ($GIT_VER >= BATCH_CMD_VER) { + _bidi_pipe($self, qw(batch-command in out pid err_c)); + $self->{-bc} = 1; + $self->{inflight} = []; + } else { + _bidi_pipe($self, qw(batch-check in_c out_c pid_c err_c)); + $self->{inflight_c} = []; + } } sub write_all { @@ -345,10 +393,18 @@ sub write_all { sub check_async ($$$$) { my ($self, $oid, $cb, $arg) = @_; - my $inflight_c = $self->{inflight_c} // check_async_begin($self); - write_all($self, $self->{out_c}, $oid."\n", - \&check_async_step, $inflight_c); - push(@$inflight_c, $oid, $cb, $arg); + my $inflight = $self->{-bc} ? + ($self->{inflight} // cat_async_begin($self)) : + ($self->{inflight_c} // check_async_begin($self)); + if ($self->{-bc}) { + substr($oid, 0, 0) = 'info '; + write_all($self, $self->{out}, "$oid\n", + \&cat_async_step, $inflight); + } else { + write_all($self, $self->{out_c}, "$oid\n", + \&check_async_step, $inflight); + } + push(@$inflight, $oid, $cb, $arg); } sub _check_cb { # check_async callback @@ -389,6 +445,8 @@ sub async_abort ($) { while (@$q) { my ($req, $cb, $arg) = splice(@$q, 0, 3); $req = $$req if ref($req); + $self->{-bc} and + $req =~ s/\A(?:contents|info) //; $req =~ s/ .*//; # drop git_dir for Gcf2Client eval { $cb->(undef, $req, undef, undef, $arg) }; warn "E: (in abort) $req: $@" if $@; @@ -461,12 +519,10 @@ sub cleanup { return 1 if $lazy && (scalar(@{$self->{inflight_c} // []}) || scalar(@{$self->{inflight} // []})); local $in_cleanup = 1; - delete $self->{async_cat}; - delete $self->{async_chk}; + delete @$self{qw(async_cat async_chk)}; async_wait_all($self); - delete $self->{inflight}; - delete $self->{inflight_c}; - _destroy($self, qw(pid rbuf in out)); + delete @$self{qw(inflight inflight_c -bc)}; + _destroy($self, qw(pid rbuf in out err_c)); _destroy($self, qw(pid_c rbuf_c in_c out_c err_c)); undef; } @@ -524,14 +580,14 @@ sub pub_urls { sub cat_async_begin { my ($self) = @_; cleanup($self) if $self->alternates_changed; - $self->batch_prepare; die 'BUG: already in async' if $self->{inflight}; - $self->{inflight} = []; + batch_prepare($self); } sub cat_async ($$$;$) { my ($self, $oid, $cb, $arg) = @_; my $inflight = $self->{inflight} // cat_async_begin($self); + substr($oid, 0, 0) = 'contents ' if $self->{-bc}; write_all($self, $self->{out}, $oid."\n", \&cat_async_step, $inflight); push(@$inflight, $oid, $cb, $arg); } diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm index 6dda7340..49a3c602 100644 --- a/lib/PublicInbox/GitAsyncCat.pm +++ b/lib/PublicInbox/GitAsyncCat.pm @@ -78,6 +78,7 @@ sub async_check ($$$$) { my ($ibx, $oidish, $cb, $arg) = @_; # $ibx may be $ctx my $git = $ibx->{git} // $ibx->git; $git->check_async($oidish, $cb, $arg); + return watch_cat($git) if $git->{-bc}; # --batch-command $git->{async_chk} //= do { my $self = bless { git => $git }, 'PublicInbox::GitAsyncCheck'; $git->{in_c}->blocking(0); @@ -97,12 +98,8 @@ sub ibx_async_prefetch { $oid .= " $git->{git_dir}\n"; return $GCF2C->gcf2_async(\$oid, $cb, $arg); # true } - } elsif ($git->{async_cat} && (my $inflight = $git->{inflight})) { - if (!@$inflight) { - print { $git->{out} } $oid, "\n" or - $git->fail("write error: $!"); - return push(@$inflight, $oid, $cb, $arg); - } + } elsif ($git->{async_cat}) { + return $git->async_prefetch($oid, $cb, $arg); } undef; } diff --git a/t/nntpd.t b/t/nntpd.t index 30233ce0..bebf4203 100644 --- a/t/nntpd.t +++ b/t/nntpd.t @@ -365,7 +365,7 @@ Date: Fri, 02 Oct 1993 00:00:00 +0000 ($^O =~ /\A(?:linux)\z/) or skip "lsof /(deleted)/ check untested on $^O", 1; my @lsof = xqx([$lsof, '-p', $td->{pid}], undef, $noerr); - my $d = [ grep(/\(deleted\)/, @lsof) ]; + my $d = [ grep(/\(deleted\)/, grep(!/batch-command\.err/, @lsof)) ]; is_deeply($d, [], 'no deleted files') or diag explain($d); }; SKIP: { test_watch($tmpdir, $host_port, $group) }; -- cgit v1.2.3-24-ge0c7