]> Sergey Matveev's repositories - public-inbox.git/commitdiff
gcf2client: split out request API from regular git
authorEric Wong <e@80x24.org>
Sun, 3 Jan 2021 02:12:06 +0000 (12:12 -1400)
committerEric Wong <e@80x24.org>
Sun, 3 Jan 2021 18:30:26 +0000 (18:30 +0000)
While Gcf2Client is designed to mimic what git-cat-file writes
to stdout, its request format is different to support requests
with a git repository path included.

We'll highlight the distinction and make the GitAsyncCat support
code easier-to-follow as a result.

Since Gcf2Client relies on DS, we can rely on DS-specific code
here, too, and use a single Unix socket instead of separate
input and output pipes, reducing memory overhead in both users
and kernel space.  Due to the interactive nature of requests and
responses, the buffer size limitations of Unix sockets on Linux
seems inconsequential here (just like it is for existing "git
cat-file --batch" use).

lib/PublicInbox/Gcf2.pm
lib/PublicInbox/Gcf2Client.pm
lib/PublicInbox/GitAsyncCat.pm
t/gcf2_client.t

index 0d5c8c5702d468d07ddf15c38a47cca8595883f4..01b83c96f684f06f85b6a292ac85eb60726f34a3 100644 (file)
@@ -75,9 +75,9 @@ sub add_alt ($$) {
        $gcf2->add_alternate($objdir);
 }
 
-# Usage: $^X -MPublicInbox::Gcf2 -e 'PublicInbox::Gcf2::loop()'
+# Usage: $^X -MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop
 # (see lib/PublicInbox/Gcf2Client.pm)
-sub loop {
+sub loop () {
        my $gcf2 = new();
        my %seen;
        STDERR->autoflush(1);
index 2022293d6b0a4684c6d72adf74e6413a426fc0bf..397774f90bf212f9c74351b8d80e0807636bff7a 100644 (file)
@@ -6,33 +6,35 @@ package PublicInbox::Gcf2Client;
 use strict;
 use parent qw(PublicInbox::DS);
 use PublicInbox::Git;
-use PublicInbox::Spawn qw(popen_rd);
-use IO::Handle ();
-use PublicInbox::Syscall qw(EPOLLONESHOT);
-use PublicInbox::DS qw(dwaitpid);
+use PublicInbox::Gcf2; # fails if Inline::C or libgit2-dev isn't available
+use PublicInbox::Spawn qw(spawn);
+use Socket qw(AF_UNIX SOCK_STREAM);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 # fields:
-#      async_cat => GitAsyncCat ref (read-only pipe)
-#      sock => writable pipe to Gcf2::loop
-#      in => pipe we read from
+#      sock => socket to Gcf2::loop
+# The rest of these fields are compatible with what PublicInbox::Git
+# uses code-sharing
 #      pid => PID of Gcf2::loop process
-#      owner_pid => process which spawned {pid}
+#      pid.owner => process which spawned {pid}
+#      in => same as {sock}, for compatibility with PublicInbox::Git
+#      inflight => array (see PublicInbox::Git)
+#      cat_rbuf => scalarref, may be non-existent or empty
 sub new  {
        my ($rdr) = @_;
        my $self = bless {}, __PACKAGE__;
        # ensure the child process has the same @INC we do:
        my $env = { PERL5LIB => join(':', @INC) };
-       my ($out_r, $out_w);
-       pipe($out_r, $out_w) or die "pipe failed: $!";
+       my ($s1, $s2);
+       socketpair($s1, $s2, AF_UNIX, SOCK_STREAM, 0) or die "socketpair $!";
        $rdr //= {};
-       $rdr->{0} = $out_r;
-       my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop()]];
-       $self->{owner_pid} = $$;
-       @$self{qw(in pid)} = popen_rd($cmd, $env, $rdr);
-       fcntl($out_w, 1031, 4096) if $^O eq 'linux'; # 1031: F_SETPIPE_SZ
-       $out_w->autoflush(1);
-       $out_w->blocking(0);
+       $rdr->{0} = $rdr->{1} = $s2;
+       my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
+       $self->{'pid.owner'} = $$;
+       $self->{pid} = spawn($cmd, $env, $rdr);
+       $s1->blocking(0);
        $self->{inflight} = [];
-       $self->SUPER::new($out_w, EPOLLONESHOT); # detect errors once
+       $self->{in} = $s1;
+       $self->SUPER::new($s1, EPOLLIN|EPOLLET);
 }
 
 sub fail {
@@ -41,43 +43,43 @@ sub fail {
        PublicInbox::Git::fail($self, @_);
 }
 
-sub cat_async ($$$;$) {
+sub gcf2_async ($$$;$) {
        my ($self, $req, $cb, $arg) = @_;
-       my $inflight = $self->{inflight};
+       my $inflight = $self->{inflight} or return $self->close;
 
        # {wbuf} is rare, I hope:
        cat_async_step($self, $inflight) if $self->{wbuf};
 
-       if (!$self->write(\"$req\n")) {
-               $self->fail("gcf2c write: $!") if !$self->{sock};
-       }
+       $self->fail("gcf2c write: $!") if !$self->write($req) && !$self->{sock};
        push @$inflight, $req, $cb, $arg;
 }
 
 # ensure PublicInbox::Git::cat_async_step never calls cat_async_retry
 sub alternates_changed {}
 
-# this is the write-only end of a pipe, DS->EventLoop will call this
+# DS->EventLoop will call this
 sub event_step {
        my ($self) = @_;
        $self->flush_write;
-       $self->close if !$self->{in}; # process died
-}
+       $self->close if !$self->{in} || !$self->{sock}; # process died
+       my $inflight = $self->{inflight};
+       if ($inflight && @$inflight) {
+               cat_async_step($self, $inflight);
+               return $self->close unless $self->{in}; # process died
 
-no warnings 'once';
+               # ok, more to do, requeue for fairness
+               $self->requeue if @$inflight || exists($self->{cat_rbuf});
+       }
+}
 
 sub DESTROY {
        my ($self) = @_;
-       delete $self->{in};
-       # GitAsyncCat::event_step may reap us with WNOHANG, too
-       my $pid = delete $self->{pid} or return;
-       if ($$ == $self->{owner_pid}) {
-               PublicInbox::DS->in_loop ? $self->close : delete($self->{sock});
-               dwaitpid $pid;
-       }
+       delete $self->{sock}; # if outside EventLoop
+       PublicInbox::Git::DESTROY($self);
 }
 
-# used by GitAsyncCat
+no warnings 'once';
+
 *cat_async_step = \&PublicInbox::Git::cat_async_step;
 
 1;
index b4d1cd6951dd56a55711bfd3988343fe3a53f6ee..7d1a13dbb48bb11fa1b68f97a4f32a1a65ccd6e0 100644 (file)
@@ -15,59 +15,53 @@ our $GCF2C; # singleton PublicInbox::Gcf2Client
 
 sub close {
        my ($self) = @_;
-
-       if (my $gitish = delete $self->{gitish}) {
-               PublicInbox::Git::cat_async_abort($gitish);
+       if (my $git = delete $self->{git}) {
+               $git->cat_async_abort;
        }
        $self->SUPER::close; # PublicInbox::DS::close
 }
 
 sub event_step {
        my ($self) = @_;
-       my $gitish = $self->{gitish} or return;
-       return $self->close if ($gitish->{in} // 0) != ($self->{sock} // 1);
-       my $inflight = $gitish->{inflight};
+       my $git = $self->{git} or return;
+       return $self->close if ($git->{in} // 0) != ($self->{sock} // 1);
+       my $inflight = $git->{inflight};
        if ($inflight && @$inflight) {
-               $gitish->cat_async_step($inflight);
+               $git->cat_async_step($inflight);
 
                # child death?
-               if (($gitish->{in} // 0) != ($self->{sock} // 1)) {
+               if (($git->{in} // 0) != ($self->{sock} // 1)) {
                        $self->close;
-               } elsif (@$inflight || exists $gitish->{cat_rbuf}) {
+               } elsif (@$inflight || exists $git->{cat_rbuf}) {
                        # ok, more to do, requeue for fairness
                        $self->requeue;
                }
-       } elsif ((my $pid = waitpid($gitish->{pid}, WNOHANG)) > 0) {
+       } elsif ((my $pid = waitpid($git->{pid}, WNOHANG)) > 0) {
                # May happen if the child process is killed by a BOFH
                # (or segfaults)
-               delete $gitish->{pid};
-               warn "E: gitish $pid exited with \$?=$?\n";
+               delete $git->{pid};
+               warn "E: git $pid exited with \$?=$?\n";
                $self->close;
        }
 }
 
 sub git_async_cat ($$$$) {
        my ($git, $oid, $cb, $arg) = @_;
-       my $gitish = $GCF2C //= eval {
-               require PublicInbox::Gcf2;
+       if ($GCF2C //= eval {
                require PublicInbox::Gcf2Client;
                PublicInbox::Gcf2Client::new();
-       } // 0; # 0: do not retry if libgit2 or Inline::C are missing
-       if ($gitish) { # Gcf2 active, {inflight} may be unset due to errors
-               $GCF2C->{inflight} or
-                       $gitish = $GCF2C = PublicInbox::Gcf2Client::new();
-               $oid .= " $git->{git_dir}";
-       } else {
-               $gitish = $git;
+       } // 0) { # 0: do not retry if libgit2 or Inline::C are missing
+               $GCF2C->gcf2_async(\"$oid $git->{git_dir}\n", $cb, $arg);
+               \undef;
+       } else { # read-only end of git-cat-file pipe
+               $git->cat_async($oid, $cb, $arg);
+               $git->{async_cat} //= do {
+                       my $self = bless { git => $git }, __PACKAGE__;
+                       $git->{in}->blocking(0);
+                       $self->SUPER::new($git->{in}, EPOLLIN|EPOLLET);
+                       \undef; # this is a true ref()
+               };
        }
-       $gitish->cat_async($oid, $cb, $arg);
-       $gitish->{async_cat} //= do {
-               # read-only end of pipe (Gcf2Client is write-only end)
-               my $self = bless { gitish => $gitish }, __PACKAGE__;
-               $gitish->{in}->blocking(0);
-               $self->SUPER::new($gitish->{in}, EPOLLIN|EPOLLET);
-               \undef; # this is a true ref()
-       };
 }
 
 # this is safe to call inside $cb, but not guaranteed to enqueue
@@ -75,9 +69,9 @@ sub git_async_cat ($$$$) {
 sub git_async_prefetch {
        my ($git, $oid, $cb, $arg) = @_;
        if ($GCF2C) {
-               if ($GCF2C->{async_cat} && !$GCF2C->{wbuf}) {
-                       $oid .= " $git->{git_dir}";
-                       return $GCF2C->cat_async($oid, $cb, $arg);
+               if (!$GCF2C->{wbuf}) {
+                       $oid .= " $git->{git_dir}\n";
+                       return $GCF2C->gcf2_async(\$oid, $cb, $arg); # true
                }
        } elsif ($git->{async_cat} && (my $inflight = $git->{inflight})) {
                # we could use MAX_INFLIGHT here w/o the halving,
index bae94c773d310c60cc0087f9ad464d7fc6fabd62..6d059cada4d84f9d3da5c2cc3421e009d4d1c33b 100644 (file)
@@ -28,7 +28,7 @@ my $err_f = "$tmpdir/err";
        PublicInbox::DS->Reset;
        open my $err, '>>', $err_f or BAIL_OUT $!;
        my $gcf2c = PublicInbox::Gcf2Client::new({ 2 => $err });
-       $gcf2c->cat_async("$tree $git_a", sub {
+       $gcf2c->gcf2_async(\"$tree $git_a\n", sub {
                my ($bref, $oid, $type, $size, $arg) = @_;
                is($oid, $tree, 'got expected OID');
                is($size, 30, 'got expected length');
@@ -44,7 +44,7 @@ my $err_f = "$tmpdir/err";
        is($estr, '', 'nothing in stderr');
 
        my $trunc = substr($tree, 0, 39);
-       $gcf2c->cat_async("$trunc $git_a", sub {
+       $gcf2c->gcf2_async(\"$trunc $git_a\n", sub {
                my ($bref, $oid, $type, $size, $arg) = @_;
                is(undef, $bref, 'missing bref is undef');
                is($oid, $trunc, 'truncated OID printed');
@@ -63,7 +63,7 @@ my $err_f = "$tmpdir/err";
        PublicInbox::DS->Reset;
        open $err, '>', $err_f or BAIL_OUT $!;
        $gcf2c = PublicInbox::Gcf2Client::new({ 2 => $err });
-       $gcf2c->cat_async("$tree $git_b", sub {
+       $gcf2c->gcf2_async(\"$tree $git_b\n", sub {
                my ($bref, $oid, $type, $size, $arg) = @_;
                is(undef, $bref, 'missing bref from alt is undef');
                $called++;
@@ -78,7 +78,7 @@ my $err_f = "$tmpdir/err";
        print $alt "$git_a/objects\n" or BAIL_OUT $!;
        close $alt or BAIL_OUT;
        my $expect = xqx(['git', "--git-dir=$git_a", qw(cat-file tree), $tree]);
-       $gcf2c->cat_async("$tree $git_a", sub {
+       $gcf2c->gcf2_async(\"$tree $git_a\n", sub {
                my ($bref, $oid, $type, $size, $arg) = @_;
                is($oid, $tree, 'oid match on alternates retry');
                is($$bref, $expect, 'tree content matched');
@@ -86,5 +86,5 @@ my $err_f = "$tmpdir/err";
        });
        $gcf2c->cat_async_step($gcf2c->{inflight});
 }
-is($called, 4, 'cat_async callbacks hit');
+is($called, 4, 'gcf2_async callbacks hit');
 done_testing;