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-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 3ED831FA19 for ; Wed, 9 Sep 2020 06:26:20 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 09/11] wwwlisting: avoid hogging event loop Date: Wed, 9 Sep 2020 06:26:16 +0000 Message-Id: <20200909062618.5940-10-e@80x24.org> In-Reply-To: <20200909062618.5940-1-e@80x24.org> References: <20200909062618.5940-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: By using the just-introduced ConfigIter class. And make ManifestJsGz a subclass of it to reduce duplication. --- lib/PublicInbox/ConfigIter.pm | 12 +++ lib/PublicInbox/ManifestJsGz.pm | 92 ++++++++---------- lib/PublicInbox/WWW.pm | 19 ++-- lib/PublicInbox/WwwListing.pm | 163 ++++++++++++++------------------ 4 files changed, 125 insertions(+), 161 deletions(-) diff --git a/lib/PublicInbox/ConfigIter.pm b/lib/PublicInbox/ConfigIter.pm index 26cc70e2..e6fa8172 100644 --- a/lib/PublicInbox/ConfigIter.pm +++ b/lib/PublicInbox/ConfigIter.pm @@ -25,4 +25,16 @@ sub event_step { PublicInbox::DS::requeue($self) if defined($section); } +# for generic PSGI servers +sub each_section { + my $self = shift; + my ($pi_cfg, $i, $cb, @arg) = @$self; + while (defined(my $section = $pi_cfg->{-section_order}->[$$i++])) { + eval { $cb->($pi_cfg, $section, @arg) }; + warn "E: $@ in ${self}::each_section" if $@; + } + eval { $cb->($pi_cfg, undef, @arg) }; + warn "E: $@ in ${self}::each_section" if $@; +} + 1; diff --git a/lib/PublicInbox/ManifestJsGz.pm b/lib/PublicInbox/ManifestJsGz.pm index 328303ce..f98d9d01 100644 --- a/lib/PublicInbox/ManifestJsGz.pm +++ b/lib/PublicInbox/ManifestJsGz.pm @@ -1,8 +1,11 @@ # Copyright (C) 2020 all contributors # License: AGPL-3.0+ + +# generates manifest.js.gz for grokmirror(1) package PublicInbox::ManifestJsGz; use strict; use v5.10.1; +use parent qw(PublicInbox::WwwListing); use Digest::SHA (); use File::Spec (); use bytes (); # length @@ -19,22 +22,12 @@ for my $mod (qw(JSON::MaybeXS JSON JSON::PP)) { $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); - }; +# called by WwwListing +sub url_regexp { + my ($ctx) = @_; + # grokmirror uses relative paths, so it's domain-dependent + # SUPER calls PublicInbox::WwwListing::url_regexp + $ctx->SUPER::url_regexp('publicInbox.grokManifest', 'match=domain'); } sub fingerprint ($) { @@ -53,7 +46,7 @@ sub fingerprint ($) { } sub manifest_add ($$;$$) { - my ($self, $ibx, $epoch, $default_desc) = @_; + my ($ctx, $ibx, $epoch, $default_desc) = @_; my $url_path = "/$ibx->{name}"; my $git_dir = $ibx->{inboxdir}; if (defined $epoch) { @@ -92,12 +85,12 @@ sub manifest_add ($$;$$) { $reference =~ s!/[^/]+/?\z!!; # basename } } - $self->{-abs2urlpath}->{$git_dir} = $url_path; + $ctx->{-abs2urlpath}->{$git_dir} = $url_path; my $modified = $git->modified; - if ($modified > $self->{-mtime}) { - $self->{-mtime} = $modified; + if ($modified > ($ctx->{-mtime} // 0)) { + $ctx->{-mtime} = $modified; } - $self->{manifest}->{$url_path} = { + $ctx->{manifest}->{$url_path} = { owner => $owner, reference => $reference, description => $desc, @@ -106,48 +99,37 @@ sub manifest_add ($$;$$) { }; } -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); +sub ibx_entry { + my ($ctx, $ibx) = @_; + eval { + if (defined(my $max = $ibx->max_git_epoch)) { + my $desc = $ibx->description; + for my $epoch (0..$max) { + manifest_add($ctx, $ibx, $epoch, $desc); } - }; - warn "E: $@" if $@; - if (my $async = $self->{psgi_env}->{'pi-httpd.async'}) { - # PublicInbox::HTTPD::Async->new - $async->(undef, undef, $self); + } else { + manifest_add($ctx, $ibx); } - return; # more steps needed - } - my $abs2urlpath = delete $self->{-abs2urlpath}; - my $wcb = delete $self->{-wcb}; - my $manifest = delete $self->{manifest}; + }; + warn "E: $@" if $@; +} + +sub hide_key { 'manifest' } + +# overrides WwwListing->psgi_triple +sub psgi_triple { + my ($ctx) = @_; + my $abs2urlpath = delete($ctx->{-abs2urlpath}) // {}; + my $manifest = delete($ctx->{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 ] ]); + [ 200, [ qw(Content-Type application/gzip), + 'Last-Modified', time2str($ctx->{-mtime}), + 'Content-Length', bytes::length($out) ], [ $out ] ] } 1; diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm index 93ab3c9d..85abf327 100644 --- a/lib/PublicInbox/WWW.pm +++ b/lib/PublicInbox/WWW.pm @@ -77,8 +77,12 @@ sub call { } # top-level indices and feeds - if ($path_info eq '/' || $path_info eq '/manifest.js.gz') { - www_listing($self)->call($env); + if ($path_info eq '/') { + require PublicInbox::WwwListing; + PublicInbox::WwwListing->response($ctx); + } elsif ($path_info eq '/manifest.js.gz') { + require PublicInbox::ManifestJsGz; + PublicInbox::ManifestJsGz->response($ctx); } elsif ($path_info =~ m!$INBOX_RE\z!o) { invalid_inbox($ctx, $1) || r301($ctx, $1); } elsif ($path_info =~ m!$INBOX_RE(?:/|/index\.html)?\z!o) { @@ -171,7 +175,6 @@ sub preload { } $self->cgit; $self->stylesheets_prepare($_) for ('', '../', '../../'); - $self->www_listing; $self->news_www; $pi_config->each_inbox(\&preload_inbox); } @@ -496,21 +499,13 @@ sub cgit { } } -sub www_listing { - my ($self) = @_; - $self->{www_listing} ||= do { - require PublicInbox::WwwListing; - PublicInbox::WwwListing->new($self); - } -} - # GET $INBOX/manifest.js.gz sub get_inbox_manifest ($$$) { my ($ctx, $inbox, $key) = @_; my $r404 = invalid_inbox($ctx, $inbox); return $r404 if $r404; require PublicInbox::ManifestJsGz; - PublicInbox::ManifestJsGz::response($ctx->{env}, [$ctx->{-inbox}]); + PublicInbox::ManifestJsGz->response($ctx); } sub get_attach { diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm index a7c7cbc1..bda2761c 100644 --- a/lib/PublicInbox/WwwListing.pm +++ b/lib/PublicInbox/WwwListing.pm @@ -5,129 +5,104 @@ # Used by PublicInbox::WWW package PublicInbox::WwwListing; use strict; -use PublicInbox::Hval qw(ascii_html prurl fmt_ts); +use PublicInbox::Hval qw(prurl fmt_ts); use PublicInbox::Linkify; use PublicInbox::GzipFilter qw(gzf_maybe); -use PublicInbox::ManifestJsGz; +use PublicInbox::ConfigIter; use bytes (); # bytes::length -sub list_all_i { - my ($ibx, $list, $hide_key) = @_; - push @$list, $ibx unless $ibx->{-hide}->{$hide_key}; -} - -sub list_all ($$$) { - my ($self, $env, $hide_key) = @_; - my $list = []; - $self->{pi_config}->each_inbox(\&list_all_i, $list, $hide_key); - $list; -} +sub ibx_entry { + my ($ctx, $ibx) = @_; + my $mtime = $ibx->modified; + my $ts = fmt_ts($mtime); + my $url = prurl($ctx->{env}, $ibx->{url}); + my $tmp = <<""; +* $ts - $url + ${\$ibx->description} -sub list_match_domain_i { - my ($ibx, $list, $hide_key, $re) = @_; - if (!$ibx->{-hide}->{$hide_key} && grep(/$re/, @{$ibx->{url}})) { - push @$list, $ibx; + if (defined(my $info_url = $ibx->{infourl})) { + $tmp .= ' ' . prurl($ctx->{env}, $info_url) . "\n"; } + push @{$ctx->{-list}}, [ $mtime, $tmp ]; } -sub list_match_domain ($$$) { - my ($self, $env, $hide_key) = @_; - my $list = []; - my $host = $env->{HTTP_HOST} // $env->{SERVER_NAME}; - $host =~ s/:[0-9]+\z//; - $self->{pi_config}->each_inbox(\&list_match_domain_i, $list, $hide_key, - qr!\A(?:https?:)?//\Q$host\E(?::[0-9]+)?/!i); - $list; -} - -sub list_404 ($$) { [] } - -# TODO: +cgit -my %VALID = ( - all => \&list_all, - 'match=domain' => \&list_match_domain, - 404 => \&list_404, -); - -sub set_cb ($$$) { - my ($pi_config, $k, $default) = @_; - my $v = $pi_config->{lc $k} // $default; - $VALID{$v} || do { - warn <<""; -`$v' is not a valid value for `$k' -$k be one of `all', `match=domain', or `404' - - $VALID{$default}; - }; +sub list_match_i { # ConfigIter callback + my ($cfg, $section, $re, $ctx) = @_; + if (defined($section)) { + return if $section !~ m!\Apublicinbox\.([^/]+)\z!; + my $ibx = $cfg->lookup_name($1) or return; + if (!$ibx->{-hide}->{$ctx->hide_key} && + grep(/$re/, @{$ibx->{url}})) { + $ctx->ibx_entry($ibx); + } + } else { # undef == "EOF" + $ctx->{-wcb}->($ctx->psgi_triple); + } } -sub new { - my ($class, $www) = @_; - my $pi_config = $www->{pi_config}; - bless { - pi_config => $pi_config, - style => $www->style("\0"), - www_cb => set_cb($pi_config, 'publicInbox.wwwListing', 404), - manifest_cb => set_cb($pi_config, 'publicInbox.grokManifest', - 'match=domain'), - }, $class; +sub url_regexp { + my ($ctx, $key, $default) = @_; + $key //= 'publicInbox.wwwListing'; + $default //= '404'; + my $v = $ctx->{www}->{pi_config}->{lc $key} // $default; +again: + if ($v eq 'match=domain') { + my $h = $ctx->{env}->{HTTP_HOST} // $ctx->{env}->{SERVER_NAME}; + $h =~ s/:[0-9]+\z//; + qr!\A(?:https?:)?//\Q$h\E(?::[0-9]+)?/!i; + } elsif ($v eq 'all') { + qr/./; + } elsif ($v eq '404') { + undef; + } else { + warn <{url}); - my $tmp = <<""; -* $ts - $url - ${\$ibx->description} - - if (defined(my $info_url = $ibx->{infourl})) { - $tmp .= ' ' . prurl($env, $info_url) . "\n"; +sub hide_key { 'www' } + +sub response { + my ($class, $ctx) = @_; + bless $ctx, $class; + my $re = $ctx->url_regexp or return $ctx->psgi_triple; + my $iter = PublicInbox::ConfigIter->new($ctx->{www}->{pi_config}, + \&list_match_i, $re, $ctx); + sub { + $ctx->{-wcb} = $_[0]; # HTTP server callback + $ctx->{env}->{'pi-httpd.async'} ? + $iter->event_step : $iter->each_section; } - $tmp; } -sub html ($$) { - my ($env, $list) = @_; +sub psgi_triple { + my ($ctx) = @_; my $h = [ 'Content-Type', 'text/html; charset=UTF-8', 'Content-Length', undef ]; - my $gzf = gzf_maybe($h, $env); + my $gzf = gzf_maybe($h, $ctx->{env}); $gzf->zmore('' . 'public-inbox listing' . '
');
 	my $code = 404;
-	if (@$list) {
+	if (my $list = $ctx->{-list}) {
 		$code = 200;
-		# Schwartzian transform since Inbox->modified is expensive
-		@$list = sort {
-			$b->[0] <=> $a->[0]
-		} map { [ $_->modified, $_ ] } @$list;
-
-		my $tmp = join("\n", map { ibx_entry(@$_, $env) } @$list);
+		# sort by ->modified
+		@$list = map { $_->[1] } sort { $b->[0] <=> $a->[0] } @$list;
+		$list = join("\n", @$list);
 		my $l = PublicInbox::Linkify->new;
-		$gzf->zmore($l->to_html($tmp));
+		$gzf->zmore($l->to_html($list));
 	} else {
 		$gzf->zmore('no inboxes, yet');
 	}
 	my $out = $gzf->zflush('
'.
-				PublicInbox::WwwStream::code_footer($env) .
-				'
'); + PublicInbox::WwwStream::code_footer($ctx->{env}) . + ''); $h->[3] = bytes::length($out); [ $code, $h, [ $out ] ]; } -# not really a stand-alone PSGI app, but maybe it could be... -sub call { - my ($self, $env) = @_; - - 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'); - PublicInbox::ManifestJsGz::response($env, $list); - } else { # / - my $list = $self->{www_cb}->($self, $env, 'www'); - html($env, $list); - } -} - 1;