Subject: Re: What's cooking in git.git (topics)

From: Jakub Narebski <[email-masked]>
Date: Sun, 17 Feb 2008 23:01:29 +0100

On Sun, 17 Feb 2008, Junio C Hamano <[email-masked]> wrote:
> Jakub Narebski <[email-masked]> writes:
> > Junio C Hamano <[email-masked]> writes:
> 
>>> * br/gitweb (Fri Feb 8 14:38:04 2008 -0200) 1 commit
>>>  + gitweb: Use the config file to set repository owner's name.
>>> 
>>> On hold per Jakub's reluctance.
>> 
>> It was tested by the author (Bruno Ribas) that it doesn't affect
>> performance, although IMHO for a bit superficial test (1000 identical
>> repositories, gitweb run as a script, dd or git-for-each-ref as an
>> additional load).  I'd like to heard from larger gitweb deployments
>> how it works with a webserver (ApacheBench or similar), with a real
>> set of repositories, and perhaps in real load conditions... of course
>> on test gitweb, not on live one.
> 
> I personally think the patch is Ok.  It would not affect really
> large and loaded sites, because the top-level project list is
> unusable for them without caching like John 'Warthog9' Hawley
> does for k.org, due to other performance bottleneck (namely,
> "the last changed timestamp") anyway.

I was worrying that it would affect gitweb performance badly, in spite 
of (slightly superficial) benchmarks[*1*] testing it is not the case.
Benchmarks were as far as I can understand for Linux, and I worry a bit 
about other operating systems (MacOS X, MS Windows,...) with a higher 
fork cost... although I have slight suspicion that finding owner (not 
just an id of an owner) of a file (from Perl) is of similar cost.

BTW. after reading the thread again the load in benchmark was always 
generated by dd, not by running bunch of git-for-each-ref on different 
refs in parallel.

[*1*] Whose benchmarks should be at least mentioned IMHO in commit 
message; also update to gitweb/README should be IMHO in the same commit 
(while $path/$project -> $git_dir could, but not necessarily should, be 
in separate cleanup patch).
 
> By the way, I think "real load conditions" and "test gitweb not
> live" are unfortunately mutually exclusive.  If it is known to
> be "test", it will not attract the same set of "real" people as
> the real one.

You can always stat, and generate test load with the same stats,
perhaps cut to the page / feature you are benchmarking / profiling.

By "read load conditions" I meant here running second gitweb with or 
without the gitweb.owner patch on the same server as live gitweb. But 
that would affect repetability of benchmark, although I'm not sure if 
it matters; it probably depends on how variable load is, on how 
sensitive benchmark is to detailed load, etc.
 
>> I also wonder if it would make sense to make it a feature,
> 
> It might make sense, but it would not probably not help much in
> practice.  It is overengineering, and I think it is spending
> efforts in wrong place in the first place.

I guess that after waiting a bit for comments you could either apply 
patches, apply patches squashed, or ask Bruno Ribas for resend.

I would then send patch making it a "feature" (where the important bit 
is 'override' part, and 'defaults' does not matter), you would then 
reject it because it is overengineering ;-) or not; but it would be in 
achives.
 
> What I personally think the most important thing that should
> happen to gitweb is to help cleaning up and updating John's
> caching version that powers k.org, and update our version to
> that.  John's version has seen a lot more real-life loads than
> any other installation of the vanilla version in git.git, and we
> should take advantage of his effort by slurping improvements
> from his.  People who are both interested and have been involved
> in gitweb might want to form a "gitweb-ng strike force" group,
> and make a consolidated effort, just like msysgit folks do to
> port our mess to Windows ;-).

This would be really nice. IIRC John said that he would try to find time 
to update kernel.org gitweb to latest, I guess sending caching patches 
for gitweb to the list (or a please pull request).

It was a bit unfortunate that J.H. split gitweb into many smaller 
modules to better understand code when adding caching support, the move 
which was not accepted for "mainline" gitweb, thus effectively creating 
a fork.

BTW it would be nice to have benchmark of different web interfaces under 
heavly load: gitweb, repo.or.cz gitweb, kernel.org gitweb, cgit, wit 
(the Ruby one), git-php, gitarella (if it is developed),...

>>> * nd/dashless (Wed Nov 28 23:21:57 2007 +0700) 1 commit
>>>  - Move all dashed-form commands to libexecdir
>>>
>>> Scheduled for 1.6.0.  I am not sure if we should merge this to
>>> 'next' before 1.5.5.  Most active people will be on 'next' and
>>> if we have this there, the resulting 1.5.5 release might end up
>>> having issues that come from differences this one introduces.
>>
>> What about making separate libexecdir and moving _helper_ scripts
>> (*--*) there first?
> 
> Why keep suggesting adding _more_ work, without any code nor
> discussion of how that would help?

I'm sorry. 

To start discussion: :-)

1. Currently tests check _built_ version:

   # Test the binaries we have just built.  The tests are kept in
   # t/ subdirectory and are run in trash subdirectory.

   It would be nice if there were a switch which would allow to test
   _installed_ (somewhere) version of git, to check for errors like
   some script not finding some command etc.

2. Where to put gitexecdir? /usr/libexec is not in FHS...

-- 
Jakub Narebski
Poland