* [PATCH 2/3] use "\&" where possible when referring to subroutines
2020-09-01 20:36 [PATCH 0/3] www: cleanups + scheduling improvements Eric Wong
2020-09-01 20:36 ` [PATCH 1/3] solver: drop warnings, modernize use v5.10.1, use SEEK_SET Eric Wong
@ 2020-09-01 20:36 ` Eric Wong
2020-09-01 20:36 ` [PATCH 3/3] www: manifest.js.gz generation no longer hogs event loop Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2020-09-01 20:36 UTC (permalink / raw)
To: meta
"*foo" is ambiguous in that it may refer to a bareword file handle;
so we'll use it where we can without triggering warnings.
PublicInbox::TestCommon::run_script_exit required dropping the
prototype, however. We'll also future-proof by dropping "use
warnings" in Cgit.pm and use the less-ambiguous "//=" in Inbox.pm
while we're in the area.
---
lib/PublicInbox/Cgit.pm | 5 ++---
lib/PublicInbox/Inbox.pm | 2 +-
lib/PublicInbox/TestCommon.pm | 4 ++--
lib/PublicInbox/WwwListing.pm | 6 +++---
t/replace.t | 8 ++++----
t/solver_git.t | 2 +-
xt/msgtime_cmp.t | 2 +-
7 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/lib/PublicInbox/Cgit.pm b/lib/PublicInbox/Cgit.pm
index 9a51b451..fb0d0e60 100644
--- a/lib/PublicInbox/Cgit.pm
+++ b/lib/PublicInbox/Cgit.pm
@@ -10,9 +10,8 @@ use strict;
use PublicInbox::GitHTTPBackend;
use PublicInbox::Git;
# not bothering with Exporter for a one-off
-*input_prepare = *PublicInbox::GitHTTPBackend::input_prepare;
-*serve = *PublicInbox::GitHTTPBackend::serve;
-use warnings;
+*input_prepare = \&PublicInbox::GitHTTPBackend::input_prepare;
+*serve = \&PublicInbox::GitHTTPBackend::serve;
use PublicInbox::Qspawn;
use PublicInbox::WwwStatic qw(r);
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 241001d3..4005954e 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -70,7 +70,7 @@ sub _cleanup_later ($) {
my ($self) = @_;
$cleanup_avail = cleanup_possible() if $cleanup_avail < 0;
return if $cleanup_avail != 1;
- $cleanup_timer ||= PublicInbox::DS::later(*cleanup_task);
+ $cleanup_timer //= PublicInbox::DS::later(\&cleanup_task);
$CLEANUP->{"$self"} = $self;
}
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index b03e93e0..42819179 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -158,7 +158,7 @@ sub _undo_redirects ($) {
# The default is 2.
our $run_script_exit_code;
sub RUN_SCRIPT_EXIT () { "RUN_SCRIPT_EXIT\n" };
-sub run_script_exit (;$) {
+sub run_script_exit {
$run_script_exit_code = $_[0] // 0;
die RUN_SCRIPT_EXIT;
}
@@ -180,7 +180,7 @@ package $pkg;
use strict;
use subs qw(exit);
-*exit = *PublicInbox::TestCommon::run_script_exit;
+*exit = \\&PublicInbox::TestCommon::run_script_exit;
sub main {
# the below "line" directive is a magic comment, see perlsyn(1) manpage
# line 1 "$f"
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index 365743cf..0be3764c 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -60,9 +60,9 @@ sub list_404 ($$) { [] }
# TODO: +cgit
my %VALID = (
- all => *list_all,
- 'match=domain' => *list_match_domain,
- 404 => *list_404,
+ all => \&list_all,
+ 'match=domain' => \&list_match_domain,
+ 404 => \&list_404,
);
sub set_cb ($$$) {
diff --git a/t/replace.t b/t/replace.t
index c4dcb89d..490e3b7b 100644
--- a/t/replace.t
+++ b/t/replace.t
@@ -179,10 +179,10 @@ EOF
}
}
-my $opt = { pre => *pad_msgs };
+my $opt = { pre => \&pad_msgs };
test_replace(2, 'basic', {});
test_replace(2, 'basic', $opt);
-test_replace(2, 'basic', $opt = { %$opt, post => *pad_msgs });
+test_replace(2, 'basic', $opt = { %$opt, post => \&pad_msgs });
test_replace(2, 'basic', $opt = { %$opt, rotate_bytes => 1 });
SKIP: {
@@ -190,9 +190,9 @@ SKIP: {
PublicInbox::Search::load_xapian() or skip 'Search::Xapian missing', 8;
for my $l (qw(medium)) {
test_replace(2, $l, {});
- $opt = { pre => *pad_msgs };
+ $opt = { pre => \&pad_msgs };
test_replace(2, $l, $opt);
- test_replace(2, $l, $opt = { %$opt, post => *pad_msgs });
+ test_replace(2, $l, $opt = { %$opt, post => \&pad_msgs });
test_replace(2, $l, $opt = { %$opt, rotate_bytes => 1 });
}
};
diff --git a/t/solver_git.t b/t/solver_git.t
index 78cc0edd..c162b605 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -41,7 +41,7 @@ $ibx->{-repo_objs} = [ $git ];
my $res;
my $solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
open my $log, '+>>', "$inboxdir/solve.log" or die "open: $!";
-my $psgi_env = { 'psgi.errors' => *STDERR, 'psgi.url_scheme' => 'http',
+my $psgi_env = { 'psgi.errors' => \*STDERR, 'psgi.url_scheme' => 'http',
'HTTP_HOST' => 'example.com' };
$solver->solve($psgi_env, $log, '69df7d5', {});
ok($res, 'solved a blob!');
diff --git a/xt/msgtime_cmp.t b/xt/msgtime_cmp.t
index 0ce3c042..aa96be4d 100644
--- a/xt/msgtime_cmp.t
+++ b/xt/msgtime_cmp.t
@@ -62,7 +62,7 @@ my $fh = $git->popen(@cat);
while (<$fh>) {
my ($oid, $type) = split / /;
next if $type ne 'blob';
- $git->cat_async($oid, *compare);
+ $git->cat_async($oid, \&compare);
}
$git->cat_async_wait;
ok(1);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] www: manifest.js.gz generation no longer hogs event loop
2020-09-01 20:36 [PATCH 0/3] www: cleanups + scheduling improvements Eric Wong
2020-09-01 20:36 ` [PATCH 1/3] solver: drop warnings, modernize use v5.10.1, use SEEK_SET Eric Wong
2020-09-01 20:36 ` [PATCH 2/3] use "\&" where possible when referring to subroutines Eric Wong
@ 2020-09-01 20:36 ` Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2020-09-01 20:36 UTC (permalink / raw)
To: meta
It's still as slow as before with hundreds/thousands of inboxes,
but at least it's fair. Future changes will allow it to be
cached and memoized with persistent HTTP servers.
---
MANIFEST | 1 +
lib/PublicInbox/ManifestJsGz.pm | 153 ++++++++++++++++++++++++++++++++
lib/PublicInbox/WWW.pm | 4 +-
lib/PublicInbox/WwwListing.pm | 117 +-----------------------
t/www_listing.t | 7 +-
5 files changed, 162 insertions(+), 120 deletions(-)
create mode 100644 lib/PublicInbox/ManifestJsGz.pm
diff --git a/MANIFEST b/MANIFEST
index 44670c7e..0e225b6a 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -157,6 +157,7 @@ lib/PublicInbox/Lock.pm
lib/PublicInbox/MDA.pm
lib/PublicInbox/MID.pm
lib/PublicInbox/MIME.pm
+lib/PublicInbox/ManifestJsGz.pm
lib/PublicInbox/Mbox.pm
lib/PublicInbox/MboxGz.pm
lib/PublicInbox/MsgIter.pm
diff --git a/lib/PublicInbox/ManifestJsGz.pm b/lib/PublicInbox/ManifestJsGz.pm
new file mode 100644
index 00000000..328303ce
--- /dev/null
+++ b/lib/PublicInbox/ManifestJsGz.pm
@@ -0,0 +1,153 @@
+# Copyright (C) 2020 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::ManifestJsGz;
+use strict;
+use v5.10.1;
+use Digest::SHA ();
+use File::Spec ();
+use bytes (); # length
+use PublicInbox::Inbox;
+use PublicInbox::Git;
+use IO::Compress::Gzip qw(gzip);
+use HTTP::Date qw(time2str);
+*try_cat = \&PublicInbox::Inbox::try_cat;
+
+our $json;
+for my $mod (qw(JSON::MaybeXS JSON JSON::PP)) {
+ eval "require $mod" or next;
+ # ->ascii encodes non-ASCII to "\uXXXX"
+ $json = $mod->new->ascii(1) and last;
+}
+
+sub response {
+ my ($env, $list) = @_;
+ $json or return [ 404, [], [] ];
+ my $self = bless {
+ -abs2urlpath => {},
+ -mtime => 0,
+ manifest => {},
+ -list => $list,
+ psgi_env => $env,
+ }, __PACKAGE__;
+
+ # PSGI server will call this immediately and give us a callback (-wcb)
+ sub {
+ $self->{-wcb} = $_[0]; # HTTP write callback
+ iterate_start($self);
+ };
+}
+
+sub fingerprint ($) {
+ my ($git) = @_;
+ # TODO: convert to qspawn for fairness when there's
+ # thousands of repos
+ my ($fh, $pid) = $git->popen('show-ref');
+ my $dig = Digest::SHA->new(1);
+ while (read($fh, my $buf, 65536)) {
+ $dig->add($buf);
+ }
+ close $fh;
+ waitpid($pid, 0);
+ return if $?; # empty, uninitialized git repo
+ $dig->hexdigest;
+}
+
+sub manifest_add ($$;$$) {
+ my ($self, $ibx, $epoch, $default_desc) = @_;
+ my $url_path = "/$ibx->{name}";
+ my $git_dir = $ibx->{inboxdir};
+ if (defined $epoch) {
+ $git_dir .= "/git/$epoch.git";
+ $url_path .= "/git/$epoch.git";
+ }
+ return unless -d $git_dir;
+ my $git = PublicInbox::Git->new($git_dir);
+ my $fingerprint = fingerprint($git) or return; # no empty repos
+
+ chomp(my $owner = $git->qx('config', 'gitweb.owner'));
+ chomp(my $desc = try_cat("$git_dir/description"));
+ utf8::decode($owner);
+ utf8::decode($desc);
+ $owner = undef if $owner eq '';
+ $desc = 'Unnamed repository' if $desc eq '';
+
+ # templates/hooks--update.sample and git-multimail in git.git
+ # only match "Unnamed repository", not the full contents of
+ # templates/this--description in git.git
+ if ($desc =~ /\AUnnamed repository/) {
+ $desc = "$default_desc [epoch $epoch]" if defined($epoch);
+ }
+
+ my $reference;
+ chomp(my $alt = try_cat("$git_dir/objects/info/alternates"));
+ if ($alt) {
+ # n.b.: GitPython doesn't seem to handle comments or C-quoted
+ # strings like native git does; and we don't for now, either.
+ my @alt = split(/\n+/, $alt);
+
+ # grokmirror only supports 1 alternate for "reference",
+ if (scalar(@alt) == 1) {
+ my $objdir = "$git_dir/objects";
+ $reference = File::Spec->rel2abs($alt[0], $objdir);
+ $reference =~ s!/[^/]+/?\z!!; # basename
+ }
+ }
+ $self->{-abs2urlpath}->{$git_dir} = $url_path;
+ my $modified = $git->modified;
+ if ($modified > $self->{-mtime}) {
+ $self->{-mtime} = $modified;
+ }
+ $self->{manifest}->{$url_path} = {
+ owner => $owner,
+ reference => $reference,
+ description => $desc,
+ modified => $modified,
+ fingerprint => $fingerprint,
+ };
+}
+
+sub iterate_start {
+ my ($self) = @_;
+ if (my $async = $self->{psgi_env}->{'pi-httpd.async'}) {
+ # PublicInbox::HTTPD::Async->new
+ $async->(undef, undef, $self);
+ } else {
+ event_step($self) while $self->{-wcb};
+ }
+}
+
+sub event_step {
+ my ($self) = @_;
+ while (my $ibx = shift(@{$self->{-list}})) {
+ eval {
+ if (defined(my $max = $ibx->max_git_epoch)) {
+ my $desc = $ibx->description;
+ for my $epoch (0..$max) {
+ manifest_add($self, $ibx, $epoch, $desc)
+ }
+ } else {
+ manifest_add($self, $ibx);
+ }
+ };
+ warn "E: $@" if $@;
+ if (my $async = $self->{psgi_env}->{'pi-httpd.async'}) {
+ # PublicInbox::HTTPD::Async->new
+ $async->(undef, undef, $self);
+ }
+ return; # more steps needed
+ }
+ my $abs2urlpath = delete $self->{-abs2urlpath};
+ my $wcb = delete $self->{-wcb};
+ my $manifest = delete $self->{manifest};
+ while (my ($url_path, $repo) = each %$manifest) {
+ defined(my $abs = $repo->{reference}) or next;
+ $repo->{reference} = $abs2urlpath->{$abs};
+ }
+ $manifest = $json->encode($manifest);
+ gzip(\$manifest => \(my $out));
+ $wcb->([ 200, [ qw(Content-Type application/gzip),
+ 'Last-Modified', time2str($self->{-mtime}),
+ 'Content-Length', bytes::length($out) ], [ $out ] ]);
+}
+
+1;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 2ea5d80d..93ab3c9d 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -509,8 +509,8 @@ sub get_inbox_manifest ($$$) {
my ($ctx, $inbox, $key) = @_;
my $r404 = invalid_inbox($ctx, $inbox);
return $r404 if $r404;
- require PublicInbox::WwwListing;
- PublicInbox::WwwListing::js($ctx->{env}, [$ctx->{-inbox}]);
+ require PublicInbox::ManifestJsGz;
+ PublicInbox::ManifestJsGz::response($ctx->{env}, [$ctx->{-inbox}]);
}
sub get_attach {
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index 0be3764c..12b0d9ad 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -5,24 +5,11 @@
# Used by PublicInbox::WWW
package PublicInbox::WwwListing;
use strict;
-use warnings;
use PublicInbox::Hval qw(ascii_html prurl fmt_ts);
use PublicInbox::Linkify;
-use PublicInbox::View;
-use PublicInbox::Inbox;
use PublicInbox::GzipFilter qw(gzf_maybe);
+use PublicInbox::ManifestJsGz;
use bytes (); # bytes::length
-use HTTP::Date qw(time2str);
-use Digest::SHA ();
-use File::Spec ();
-use IO::Compress::Gzip qw(gzip);
-*try_cat = \&PublicInbox::Inbox::try_cat;
-our $json;
-for my $mod (qw(JSON::MaybeXS JSON JSON::PP)) {
- eval "require $mod" or next;
- # ->ascii encodes non-ASCII to "\uXXXX"
- $json = $mod->new->ascii(1) and last;
-}
sub list_all_i {
my ($ibx, $arg) = @_;
@@ -132,106 +119,6 @@ sub html ($$) {
[ $code, $h, [ $out ] ];
}
-sub fingerprint ($) {
- my ($git) = @_;
- # TODO: convert to qspawn for fairness when there's
- # thousands of repos
- my ($fh, $pid) = $git->popen('show-ref');
- my $dig = Digest::SHA->new(1);
- while (read($fh, my $buf, 65536)) {
- $dig->add($buf);
- }
- close $fh;
- waitpid($pid, 0);
- return if $?; # empty, uninitialized git repo
- $dig->hexdigest;
-}
-
-sub manifest_add ($$;$$) {
- my ($manifest, $ibx, $epoch, $default_desc) = @_;
- my $url_path = "/$ibx->{name}";
- my $git_dir = $ibx->{inboxdir};
- if (defined $epoch) {
- $git_dir .= "/git/$epoch.git";
- $url_path .= "/git/$epoch.git";
- }
- return unless -d $git_dir;
- my $git = PublicInbox::Git->new($git_dir);
- my $fingerprint = fingerprint($git) or return; # no empty repos
-
- chomp(my $owner = $git->qx('config', 'gitweb.owner'));
- chomp(my $desc = try_cat("$git_dir/description"));
- utf8::decode($owner);
- utf8::decode($desc);
- $owner = undef if $owner eq '';
- $desc = 'Unnamed repository' if $desc eq '';
-
- # templates/hooks--update.sample and git-multimail in git.git
- # only match "Unnamed repository", not the full contents of
- # templates/this--description in git.git
- if ($desc =~ /\AUnnamed repository/) {
- $desc = "$default_desc [epoch $epoch]" if defined($epoch);
- }
-
- my $reference;
- chomp(my $alt = try_cat("$git_dir/objects/info/alternates"));
- if ($alt) {
- # n.b.: GitPython doesn't seem to handle comments or C-quoted
- # strings like native git does; and we don't for now, either.
- my @alt = split(/\n+/, $alt);
-
- # grokmirror only supports 1 alternate for "reference",
- if (scalar(@alt) == 1) {
- my $objdir = "$git_dir/objects";
- $reference = File::Spec->rel2abs($alt[0], $objdir);
- $reference =~ s!/[^/]+/?\z!!; # basename
- }
- }
- $manifest->{-abs2urlpath}->{$git_dir} = $url_path;
- my $modified = $git->modified;
- if ($modified > $manifest->{-mtime}) {
- $manifest->{-mtime} = $modified;
- }
- $manifest->{$url_path} = {
- owner => $owner,
- reference => $reference,
- description => $desc,
- modified => $modified,
- fingerprint => $fingerprint,
- };
-}
-
-# manifest.js.gz
-sub js ($$) {
- my ($env, $list) = @_;
- # $json won't be defined if IO::Compress::Gzip is missing
- $json or return [ 404, [], [] ];
-
- my $manifest = { -abs2urlpath => {}, -mtime => 0 };
- for my $ibx (@$list) {
- if (defined(my $max = $ibx->max_git_epoch)) {
- my $desc = $ibx->description;
- for my $epoch (0..$max) {
- manifest_add($manifest, $ibx, $epoch, $desc);
- }
- } else {
- manifest_add($manifest, $ibx);
- }
- }
- my $abs2urlpath = delete $manifest->{-abs2urlpath};
- my $mtime = delete $manifest->{-mtime};
- while (my ($url_path, $repo) = each %$manifest) {
- defined(my $abs = $repo->{reference}) or next;
- $repo->{reference} = $abs2urlpath->{$abs};
- }
- my $out;
- gzip(\($json->encode($manifest)) => \$out);
- $manifest = undef;
- [ 200, [ qw(Content-Type application/gzip),
- 'Last-Modified', time2str($mtime),
- 'Content-Length', bytes::length($out) ], [ $out ] ];
-}
-
# not really a stand-alone PSGI app, but maybe it could be...
sub call {
my ($self, $env) = @_;
@@ -239,7 +126,7 @@ sub call {
if ($env->{PATH_INFO} eq '/manifest.js.gz') {
# grokmirror uses relative paths, so it's domain-dependent
my $list = $self->{manifest_cb}->($self, $env, 'manifest');
- js($env, $list);
+ PublicInbox::ManifestJsGz::response($env, $list);
} else { # /
my $list = $self->{www_cb}->($self, $env, 'www');
html($env, $list);
diff --git a/t/www_listing.t b/t/www_listing.t
index c4511cd1..4309a5e1 100644
--- a/t/www_listing.t
+++ b/t/www_listing.t
@@ -10,9 +10,10 @@ use PublicInbox::Import;
require_mods(qw(URI::Escape Plack::Builder Digest::SHA
IO::Compress::Gzip IO::Uncompress::Gunzip HTTP::Tiny));
require PublicInbox::WwwListing;
+require PublicInbox::ManifestJsGz;
my $json = do {
no warnings 'once';
- $PublicInbox::WwwListing::json;
+ $PublicInbox::ManifestJsGz::json;
} or plan skip_all => "JSON module missing";
use_ok 'PublicInbox::Git';
@@ -20,7 +21,7 @@ use_ok 'PublicInbox::Git';
my ($tmpdir, $for_destroy) = tmpdir();
my $bare = PublicInbox::Git->new("$tmpdir/bare.git");
PublicInbox::Import::init_bare($bare->{git_dir});
-is(PublicInbox::WwwListing::fingerprint($bare), undef,
+is(PublicInbox::ManifestJsGz::fingerprint($bare), undef,
'empty repo has no fingerprint');
{
my $fi_data = './t/git.fast-import-data';
@@ -30,7 +31,7 @@ is(PublicInbox::WwwListing::fingerprint($bare), undef,
'fast-import');
}
-like(PublicInbox::WwwListing::fingerprint($bare), qr/\A[a-f0-9]{40}\z/,
+like(PublicInbox::ManifestJsGz::fingerprint($bare), qr/\A[a-f0-9]{40}\z/,
'got fingerprint with non-empty repo');
sub tiny_test {
^ permalink raw reply related [flat|nested] 4+ messages in thread