[PATCH 1/3] xap_helper_cxx: use write_file helper

[PATCH 1/3] xap_helper_cxx: use write_file helper

[Eric Wong]

PublicInbox::IO already gets loaded by PublicInbox::Spawn, so
there's no avoiding it even if we want fast startup time :<
---
 lib/PublicInbox/XapHelperCxx.pm | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 908a71f4..090ac9b7 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -8,6 +8,7 @@
 package PublicInbox::XapHelperCxx;
 use v5.12;
 use PublicInbox::Spawn qw(run_qx which);
+use PublicInbox::IO qw(read_all write_file);
 use PublicInbox::Search;
 use Fcntl qw(SEEK_SET);
 use Config;
@@ -39,11 +40,11 @@ EOM
 
 sub needs_rebuild () {
 	open my $fh, '<', "$dir/XFLAGS" or return 1;
-	chomp(my $prev = <$fh>);
+	chomp(my $prev = read_all($fh));
 	return 1 if $prev ne $xflags;
 
 	open $fh, '<', "$dir/xap_modversion" or return 1;
-	chomp($prev = <$fh>);
+	chomp($prev = read_all($fh));
 	$prev or return 1;
 
 	$xap_modversion = xap_cfg('--modversion');
@@ -62,11 +63,11 @@ sub build () {
 	my ($prog) = ($bin =~ m!/([^/]+)\z!);
 	my $tmp = File::Temp->newdir(DIR => $dir) // die "newdir: $!";
 	my $src = "$tmp/$prog.cpp";
-	open my $fh, '>', $src;
+	my $fh = write_file '>', $src;
 	for (@srcs) {
 		say $fh qq(# line 1 "$_");
 		open my $rfh, '<', $_;
-		print $fh PublicInbox::IO::read_all $rfh;
+		print $fh read_all($rfh);
 	}
 	print $fh PublicInbox::Search::generate_cxx();
 	print $fh PublicInbox::CodeSearch::generate_cxx();
@@ -86,13 +87,8 @@ sub build () {
 
 	my $cmd = "$cxx $src $fl $xflags -o $tmp/$prog";
 	system($cmd) and die "$cmd failed: \$?=$?";
-	open $fh, '>', "$tmp/XFLAGS";
-	say $fh $xflags;
-	close $fh;
-
-	open $fh, '>', "$tmp/xap_modversion";
-	say $fh $xap_modversion;
-	close $fh;
+	write_file '>', "$tmp/XFLAGS", $xflags, "\n";
+	write_file '>', "$tmp/xap_modversion", $xap_modversion, "\n";
 	undef $xap_modversion; # do we ever build() twice?
 	# not quite atomic, but close enough :P
 	rename("$tmp/$_", "$dir/$_") for ($prog, qw(XFLAGS xap_modversion));

[PATCH 2/3] xap_helper_cxx: make the build process ccache-friendly

[Eric Wong]

We need to have stable filenames and separate compilation
and linkage stages for ccache to hit.
---
 lib/PublicInbox/XapHelperCxx.pm | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 090ac9b7..88a83438 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -7,7 +7,7 @@
 # The resulting executable is not linked to Perl in any way.
 package PublicInbox::XapHelperCxx;
 use v5.12;
-use PublicInbox::Spawn qw(run_qx which);
+use PublicInbox::Spawn qw(run_die run_qx which);
 use PublicInbox::IO qw(read_all write_file);
 use PublicInbox::Search;
 use Fcntl qw(SEEK_SET);
@@ -24,8 +24,8 @@ my @pm_dep = map { $srcpfx.$_ } qw(Search.pm CodeSearch.pm);
 my $ldflags = '-Wl,-O1';
 $ldflags .= ' -Wl,--compress-debug-sections=zlib' if $^O ne 'openbsd';
 my $xflags = ($ENV{CXXFLAGS} // '-Wall -ggdb3 -O0') . ' ' .
-	($ENV{LDFLAGS} // $ldflags) .
-	qq{ -DTHREADID=}.PublicInbox::Search::THREADID;
+	qq{ -DTHREADID=}.PublicInbox::Search::THREADID . ' ' .
+	($ENV{LDFLAGS} // $ldflags);
 my $xap_modversion;
 
 sub xap_cfg (@) {
@@ -58,12 +58,12 @@ sub build () {
 		die "mkdir($dir): $err" if !-d $dir;
 	}
 	use autodie;
-	require File::Temp;
 	require PublicInbox::CodeSearch;
+	require PublicInbox::Lock;
+	require PublicInbox::OnDestroy;
 	my ($prog) = ($bin =~ m!/([^/]+)\z!);
-	my $tmp = File::Temp->newdir(DIR => $dir) // die "newdir: $!";
-	my $src = "$tmp/$prog.cpp";
-	my $fh = write_file '>', $src;
+	my $lk = PublicInbox::Lock->new("$dir/$prog.lock")->lock_for_scope;
+	my $fh = write_file '>', "$dir/$prog.cpp";
 	for (@srcs) {
 		say $fh qq(# line 1 "$_");
 		open my $rfh, '<', $_;
@@ -73,6 +73,10 @@ sub build () {
 	print $fh PublicInbox::CodeSearch::generate_cxx();
 	close $fh;
 
+	opendir my $dh, '.';
+	my $restore = PublicInbox::OnDestroy->new(\&chdir, $dh);
+	chdir $dir;
+
 	# xap_modversion may be set by needs_rebuild
 	$xap_modversion //= xap_cfg('--modversion');
 	my $fl = xap_cfg(qw(--libs --cflags));
@@ -84,14 +88,15 @@ sub build () {
 	# distributed packages.
 	$^O eq 'netbsd' and $fl =~ s/(\A|[ \t])\-L([^ \t]+)([ \t]|\z)/
 				"$1-L$2 -Wl,-rpath=$2$3"/egsx;
-
-	my $cmd = "$cxx $src $fl $xflags -o $tmp/$prog";
-	system($cmd) and die "$cmd failed: \$?=$?";
-	write_file '>', "$tmp/XFLAGS", $xflags, "\n";
-	write_file '>', "$tmp/xap_modversion", $xap_modversion, "\n";
+	my @xflags = split(/\s+/, "$fl $xflags");
+	my @cflags = grep(!/\A-(?:Wl|l|L)/, @xflags);
+	run_die([$cxx, '-c', "$prog.cpp", @cflags]);
+	run_die([$cxx, '-o', "$prog.tmp", "$prog.o", @xflags]);
+	write_file '>', 'XFLAGS.tmp', $xflags, "\n";
+	write_file '>', 'xap_modversion.tmp', $xap_modversion, "\n";
 	undef $xap_modversion; # do we ever build() twice?
 	# not quite atomic, but close enough :P
-	rename("$tmp/$_", "$dir/$_") for ($prog, qw(XFLAGS xap_modversion));
+	rename("$_.tmp", $_) for ($prog, qw(XFLAGS xap_modversion));
 }
 
 sub check_build () {

[PATCH 3/3] xap_client: spawn C++ xap_helper directly

[Eric Wong]

No need to suffer through slow Perl load times when we can build
in the parent Perl process and get the executable path name to
pass to spawn directly.
---
 lib/PublicInbox/XapClient.pm    | 28 ++++++++++++++--------------
 lib/PublicInbox/XapHelperCxx.pm |  9 ++++-----
 t/xap_helper.t                  | 22 +++++++++-------------
 3 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/lib/PublicInbox/XapClient.pm b/lib/PublicInbox/XapClient.pm
index 21c89265..dda5e044 100644
--- a/lib/PublicInbox/XapClient.pm
+++ b/lib/PublicInbox/XapClient.pm
@@ -11,7 +11,7 @@ use v5.12;
 use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_SEQPACKET);
 use PublicInbox::IPC;
-use autodie qw(pipe socketpair);
+use autodie qw(fork pipe socketpair);
 
 sub mkreq {
 	my ($self, $ios, @arg) = @_;
@@ -28,19 +28,19 @@ sub mkreq {
 sub start_helper {
 	my @argv = @_;
 	socketpair(my $sock, my $in, AF_UNIX, SOCK_SEQPACKET, 0);
-	my $cls = ($ENV{PI_NO_CXX} ? undef : eval {
-			require PublicInbox::XapHelperCxx;
-			PublicInbox::XapHelperCxx::check_build();
-			'PublicInbox::XapHelperCxx';
-		}) // do {
-			require PublicInbox::XapHelper;
-			'PublicInbox::XapHelper';
-		};
-	# ensure the child process has the same @INC we do:
-	my $env = { PERL5LIB => join(':', @INC) };
-	my $pid = spawn([$^X, ($^W ? ('-w') : ()), "-M$cls", '-e',
-				$cls.'::start(@ARGV)', '--', @argv],
-			$env, { 0 => $in });
+	require PublicInbox::XapHelperCxx;
+	my $cls = 'PublicInbox::XapHelperCxx';
+	my $env;
+	my $cmd = eval { PublicInbox::XapHelperCxx::cmd() };
+	if ($@) { # fall back to Perl + XS|SWIG
+		require PublicInbox::XapHelper;
+		$cls = 'PublicInbox::XapHelper';
+		# ensure the child process has the same @INC we do:
+		$env = { PERL5LIB => join(':', @INC) };
+		$cmd = [$^X, ($^W ? ('-w') : ()), "-M$cls", '-e',
+			$cls.'::start(@ARGV)', '--' ];
+	}
+	my $pid = spawn($cmd, $env, { 0 => $in });
 	((bless { io => $sock, impl => $cls }, __PACKAGE__), $pid);
 }
 
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index 88a83438..e257fb6e 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -113,17 +113,16 @@ sub check_build () {
 	needs_rebuild() ? build() : 0;
 }
 
-sub start (@) {
+# returns spawn arg
+sub cmd {
 	check_build();
 	my @cmd;
 	if (my $v = $ENV{VALGRIND}) {
 		$v = 'valgrind -v' if $v eq '1';
 		@cmd = split(/\s+/, $v);
 	}
-	push @cmd, $bin, @_;
-	my $prog = $cmd[0];
-	$cmd[0] =~ s!\A.*?/([^/]+)\z!$1!;
-	exec { $prog } @cmd;
+	push @cmd, $bin;
+	\@cmd;
 }
 
 1;
diff --git a/t/xap_helper.t b/t/xap_helper.t
index 7890392d..83f59d7d 100644
--- a/t/xap_helper.t
+++ b/t/xap_helper.t
@@ -61,11 +61,11 @@ my $doreq = sub {
 
 my $env = { PERL5LIB => join(':', @INC) };
 my $test = sub {
-	my (@arg) = @_;
+	my (@cmd) = @_;
 	socketpair(my $s, my $y, AF_UNIX, SOCK_SEQPACKET, 0);
-	my $pid = spawn([$^X, '-w', @arg], $env, { 0 => $y });
+	my $pid = spawn(\@cmd, $env, { 0 => $y });
 	my $ar = PublicInbox::AutoReap->new($pid);
-	diag "$arg[-1] running pid=$pid";
+	diag "$cmd[-1] running pid=$pid";
 	close $y;
 	my $r = $doreq->($s, qw(test_inspect -d), $ibx_idx[0]);
 	my %info = map { split(/=/, $_, 2) } split(/ /, do { local $/; <$r> });
@@ -141,24 +141,20 @@ my $test = sub {
 
 my @NO_CXX = (1);
 unless ($ENV{TEST_XH_CXX_ONLY}) {
-	my $ar = $test->(qw[-MPublicInbox::XapHelper -e
+	my $ar = $test->($^X, qw[-w -MPublicInbox::XapHelper -e
 			PublicInbox::XapHelper::start('-j0')]);
-	($ar, my $s) = $test->(qw[-MPublicInbox::XapHelper -e
+	($ar, my $s) = $test->($^X, qw[-w -MPublicInbox::XapHelper -e
 			PublicInbox::XapHelper::start('-j1')]);
 	no_pollerfd($ar->{pid});
 }
 SKIP: {
-	eval {
-		require PublicInbox::XapHelperCxx;
-		PublicInbox::XapHelperCxx::check_build();
-	};
+	require PublicInbox::XapHelperCxx;
+	my $cmd = eval { PublicInbox::XapHelperCxx::cmd() };
 	skip "XapHelperCxx build: $@", 1 if $@ || $ENV{PI_NO_CXX};
 
 	@NO_CXX = $ENV{TEST_XH_CXX_ONLY} ? (0) : (0, 1);
-	my $ar = $test->(qw[-MPublicInbox::XapHelperCxx -e
-			PublicInbox::XapHelperCxx::start('-j0')]);
-	$ar = $test->(qw[-MPublicInbox::XapHelperCxx -e
-			PublicInbox::XapHelperCxx::start('-j1')]);
+	my $ar = $test->(@$cmd, '-j0');
+	$ar = $test->(@$cmd, '-j1');
 };
 
 require PublicInbox::CodeSearch;