From: Peter Vereshagin <peter@vereshagin•org>
To: Jakub Narebski <jnareb@gmail•com>
Cc: Petr Baudis <pasky@suse•cz>, Eric Wong <normalperson@yhbt•net>,
git@vger•kernel.org, Sam Vilain <sam.vilain@catalyst•net.nz>,
Juan Jose Comellas <juanjo@comellas•org>,
John Goerzen <jgoerzen@complete•org>
Subject: Re: [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script
Date: Tue, 11 May 2010 10:24:15 +0400 [thread overview]
Message-ID: <20100511062415.GA5220@screwed.box> (raw)
In-Reply-To: <201005101729.07334.jnareb@gmail.com>
I know St. Peter won't call your name, Jakub!
2010/05/10 17:29:03 +0200 Jakub Narebski <jnareb@gmail•com> => To Peter Vereshagin :
JN> On Mon, 10 May 2010, Peter Vereshagin wrote:
JN> > 2010/05/09 20:18:52 +0200 Jakub Narebski <jnareb@gmail•com> => To Peter Vereshagin :
JN> >
JN> > Great! I was just about to ask on caching, etc. What a complex history on all
JN> > of that, will be on those tracks after some of my whiles. ;-)
JN>
JN> You can find current state of my take on gitweb output caching (based
JN> on / inspired by work by John 'Warthog9' Hawley) in my repository on
JN> repo.or.cz, in the 'gitweb/cache-kernel-pu' branch:
JN>
JN> http://repo.or.cz/w/git/jnareb-git.git gitweb/cache-kernel-pu
JN>
JN> You can find progress reports (and what current show-stoppers are) in
JN> git mailing list archives.
I will.
JN> Note that http://repo.or.cz does its own gitweb caching, IIRC by
JN> caching Perl data, and only for 'projects_list' page (the most costly
JN> one).
JN>
JN> There was also "Gitweb caching" projects in GSoC 2008 by Lea Wiemann,
JN> which IIUC cached output of git commands. This project was, I think,
JN> completed but didn't get merged into git.
JN>
JN> > JN> What are required changes to gitweb to use FCGI::Spawn to run gitweb as
JN> > JN> a FastCGI script? Alternatively, how the wrapper script for gitweb
JN> > JN> (gitweb.fcgi) to be run as FastCGI should look like to use FCGI::Spawn?
JN> >
JN> > By far it's only an exit() of the what I use (1.6.0.6):
JN>
JN> Why so old git? Current version is git version 1.7.1
Update or die sounds too consumerical to me ;-)
JN>
JN> >
JN> > --- /usr/local/share/examples/git/gitweb/gitweb.cgi 2010-02-25 13:49:30.068287112 +0300
JN> > +++ www/gitweb.cgi 2010-03-13 14:28:45.326244103 +0300
JN>
JN> Hrmph. Why not use "git diff --no-index <file1> <file2>" here?
JN>
JN> The Perl-aware equivalent of '-p' option of GNU diff, i.e. showing in
JN> which function we are in hunk headers, would help here.
It's obvious that I just made a more simple thing about this patch: grep exit
gitweb.cgi. Meat and potatoes ;-)
JN>
JN> > @@ -933,7 +933,7 @@
JN> > die_error(400, "Project needed");
JN> > }
JN> > $actions{$action}->();
JN> > -exit;
JN> > +# exit;
JN>
JN> This 'exit' was here just in case there were some forgotten code below
JN> this line outside subroutines (that should not be run). It can be
JN> safely removed.
JN>
JN> >
JN> > ## ======================================================================
JN> > ## action links
JN> > @@ -3371,7 +3371,7 @@ sub die_error {
JN>
JN> I have added my guess of in which subroutine this code is above.
Right.
JN>
JN> > </div>
JN> > EOF
JN> > git_footer_html();
JN> > - exit;
JN> > +# exit;
JN> > }
JN>
JN> Err... and gitweb works correctly with this change? This 'exit' was
JN> required for die_error to function like 'die' in that it finishes
JN> serving request, and should not continue subroutine it was called
JN> from.
Does at least on 'non-existent diff' page:
http://gitweb.vereshagin.org/fcgiproxy/commitdiff/abcd
JN> I have changed this 'exit' to non-local goto to toplevel. It could be
JN> done instead by redefining 'exit' subroutine, like shown below, but I
JN> feel that would be hacky if you can change gitweb code (it is not
JN> black box you should not touch).
Right, one shouldn't ever redefine perl built-in functions. I did only because
of no other way to 'get things working'
JN>
JN> >
JN> > ## ----------------------------------------------------------------------
JN> >
JN> > but it's probably even not necessary with -e parameter:
JN> > http://search.cpan.org/~veresc/FCGI-Spawn-0.16.1/fcgi_spawn#Command_line_options
JN> > which is definitely required for bugzilla, the worst boy in that sandbox. The
JN> > parameter does just this:
JN> > ===
JN> > my $cref = sub {
JN> > if ('FCGI::ProcManager' eq scalar caller) {
JN> > CORE::exit @_;
JN> > } else {
JN> > no warnings;
JN> > last CALLED_OUT;
JN> > }
JN> > };
JN> > *CORE::GLOBAL::exit = $cref;
JN> > *CORE::GLOBAL::exit;
JN> > ===
JN>
JN> This is quite nice idea to replace 'exit' by subroutine that does
JN> non-local jump to outside of application, at the end of request loop.
JN> Such "monkey patching" is the only solution if you can't or shouldn't
JN> modify application code (like FCGI::Spawn being generic solution).
Yes, this is quick-n-dirty to apply for those monkeys who are just busy to care
about re-writing CGI apps.
JN> > so this requires configuration
JN> > ( $PREFIX/etc/fcgi_spawn/preload_nonprepared_01.pl, in my case ) for fcgi_spawn
JN> > daemon like this:
JN> > ===
JN> > $spawn->{ callout } = sub{ do shift;
JN> > CALLED_OUT:
JN> > };
JN> > ===
JN>
JN> Here
JN>
JN> $spawn->{'callout'} = sub {
JN> my $cgi_app = shift;
JN> do $cgi_app;
JN>
JN> # this is needed for sane error handling
JN> die "Couldn't parse $cgi_app: $@" if $@;
JN>
JN> CALLED_OUT:
JN> };
in a forked application, die() is a PITA on any reasonable load. It makes the
CoW-shared memory to be copied into separate area and being marked as unusable
before the process is dead. This is the only case I saw load averages on the
servers valued as crazy ~700.
So just exit there, not die. By far, die can not be redefined the same way as I
propose for exit in FCGI::Spawn.
JN>
JN> could be simply replaced by
JN>
JN> use CGI::Compile;
JN>
JN> # ...
JN>
JN> $spawn->{'callout'} = \&{CGI::Compile->compile}
JN>
JN> or something like that. See CGI::Compile manpage and CGI::Compile source:
JN> http://cpansearch.perl.org/src/MIYAGAWA/CGI-Compile-0.11/lib/CGI/Compile.pm
JN>
JN> >
JN> > All of that is not needed without exit() in gitweb, now.
JN>
JN> BTW I wonder what are the consequences for performance on replacing
JN> 'exit' by non-local jump. It can degrade performance a bit for gitweb
JN> run as pure CGI (mod_cgi / mod_cgid), but should improve performance
JN> for mod_perl, at least if there are more connections... unless
JN> ModPerl::Registry does similar trick with exit().
I knew out about such a trick somewhere in modperlbook. For mod_perl, this
should be done in startup.pl
JN> > I didn't mean FCGI::PM is a problem by itself. The standalone gitweb daemon is
JN> > great thing for those who need such a choice. FCGI::Spawn is just for some
JN> > different task: to put several ( wish to say: any CGI app ) applications inside
JN> > the same fork()ed processes. It should be just obviously documented for a user
JN> > as a dependency for implementation of a gitweb fastcgi daemon. Although I'm not
JN> > sure if the FCGI::PM package should be a dependency for git package for any OS:
JN> > for those modules use()d in eval() my guess is: particular user's choice to be
JN> > offered.
JN> >
JN> > So FCGI::PM usage I think makes a flavor taste for any daemon and thus should
JN> > be explicit. YMMV for those uninvolved in daemonizing, of course. ;-)
JN>
JN> Hmmm... is FCGI::Spawn really needed, or can it be replaced by simple
JN> PSGI wrapper using either Plack::App::CGIBin,
JN>
JN> use Plack::App::CGIBin;
JN> use Plack::Builder;
JN>
JN> my $app = Plack::App::CGIBin->new(root => "/path/to/cgi-bin")->to_app;
JN> builder {
JN> mount "/cgi-bin" => $app;
JN> };
You use the predefined paths here on initialization. FCGI::Spawn knows about
the CGI application's path at the right moment it takes the request.
JN> or Plack::App::WrapCGI plus Plack::App::URLMap, the last indirectly
JN> via Plack::Builder DSL:
JN>
JN> use Plack::Builder;
JN> use Plack::App::WrapCGI;
JN>
JN> builder {
JN> mount "/foo" =>
JN> Plack::App::WrapCGI->new(script => "foo.cgi")->to_app;
JN> mount "/bar" =>
JN> Plack::App::WrapCGI->new(script => "bar.cgi")->to_app;
JN> };
Sounds no more simple than simplicity of php deployment. That is whom the
FCGI::Spawn combats for.
Probably, 'the directories and scripts cache' should help to defer such an
initialization? like it is done about the DBI handles in Apache::DBI. You can
perform that init on a first request per fork, and keep it built for all of the
process lifetime for the requests coming next.
JN> > Is it probable that gitweb doesn't take any POSTs requests? The main trick
JN> > around FCGI::Spawn is the need to patch the CGI.pm but if that is the case...
JN> > I'd try to redefine the STDIN to /dev/null or zero so FCGI.Spawn.CGI.pm.patch
JN> > should be unnecessary for one who only wants to run the gitweb in FCGI::Spawn.
JN> > If switch to FCGI.pm will be way complicated to me.
JN>
JN> Errr... excuse me, what you wanted to say in the paragraph above?
CGI::Fast use CGI.pm for POST input. With FCGI.pm I'll use CGI.pm for POST
input only on an application's demand in FCGI::Spawn.
In case of gitweb.cgi POST is not used. This makes the CGI.pm patch supplied
with FCGI::Spawn not needed.
But web user may send the POST request and FCGI::Spawn should feed a dummy
input for CGI.pm in the CGI script waiting for input from STDIN when request
method is POST.
Not sure if this feature is needed at all for FCGI::Spawn though.
JN> Gitweb doesn't use no POST requests: it is read-only web repository
JN> browser... well, except for the 'show_ctags' action.
Tag cloud? Is there an example of usable tag cloud on any public gitweb out
there?
73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627)
--
http://vereshagin.org
next prev parent reply other threads:[~2010-05-11 6:24 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-07 12:54 [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski
2010-05-07 12:54 ` [PATCH/RFC 1/2] gitweb: Put all per-connection code in run() subroutine Jakub Narebski
2010-05-07 12:54 ` [RFC/PATCH 2/2] gitweb: Add support for FastCGI, using CGI::Fast Jakub Narebski
2010-05-08 7:59 ` [RFC/PATCHv2 " Jakub Narebski
2010-05-08 22:41 ` [PATCH 0/2] gitweb: Add support for running gitweb as FastCGI script Jakub Narebski
2010-05-09 9:31 ` Eric Wong
2010-05-09 11:48 ` Ævar Arnfjörð Bjarmason
2010-05-09 12:39 ` Jakub Narebski
2010-05-09 16:47 ` Peter Vereshagin
2010-05-09 18:18 ` Jakub Narebski
2010-05-10 7:13 ` Peter Vereshagin
2010-05-10 15:29 ` Jakub Narebski
2010-05-11 6:24 ` Peter Vereshagin [this message]
2010-05-11 8:35 ` Petr Baudis
2010-05-11 10:58 ` Jakub Narebski
2010-05-11 12:09 ` Peter Vereshagin
2010-05-11 13:51 ` Jakub Narebski
2010-05-13 13:10 ` Peter Vereshagin
2010-05-13 17:13 ` Ævar Arnfjörð Bjarmason
2010-05-14 15:58 ` Peter Vereshagin
2010-05-14 10:53 ` Jakub Narebski
2010-05-14 15:36 ` Peter Vereshagin
2010-05-14 17:58 ` Jakub Narebski
2010-05-14 18:43 ` Jakub Narebski
2010-05-15 10:06 ` Peter Vereshagin
2010-05-15 13:58 ` Jakub Narebski
2010-05-16 10:15 ` Peter Vereshagin
2010-05-18 1:06 ` Jakub Narebski
2010-05-16 10:26 ` Petr Baudis
2010-05-15 11:51 ` Petr Baudis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100511062415.GA5220@screwed.box \
--to=peter@vereshagin$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=jgoerzen@complete$(echo .)org \
--cc=jnareb@gmail$(echo .)com \
--cc=juanjo@comellas$(echo .)org \
--cc=normalperson@yhbt$(echo .)net \
--cc=pasky@suse$(echo .)cz \
--cc=sam.vilain@catalyst$(echo .)net.nz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox