Re: [PATCHv6 10/10] gitweb: group remote heads by remote
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Nov 8, 2010, 13:41 UTC
- Message-ID
- <201011081441.44572.jnareb@gmail.com>
- In-Reply-To
- <AANLkTikK1C5zzfOKSucPhOAQV3E=mrOunpyv4NhN+Od6@mail.gmail.com>
On Mon, Nov 8, 2010, Giuseppe Bilotta napisał:
> On Mon, Nov 8, 2010 at 12:05 PM, Jakub Narebski <jnareb@gmail.com> wrote: >> On Mon, 8 Nov 2010, Giuseppe Bilotta wrote: >>> On Thu, Nov 4, 2010 at 11:41 AM, Jakub Narebski <jnareb@gmail.com> wrote: >>>> On Wed, 3 Nov 2010, Giuseppe Bilotta wrote:
[...]
Show 21 quoted lines
>>>> BTW. would next version of this series include patch to git-instaweb
>>>> enabling 'remote_heads' feature for it (gitweb_conf function)?
>>>
>>> I will look into that.
>>
>> It can be as simple as
>>
>> diff --git i/git-instaweb.sh w/git-instaweb.sh
>> index e6f6ecd..50f65b1 100755
>> --- i/git-instaweb.sh
>> +++ w/git-instaweb.sh
>> @@ -580,6 +580,8 @@ gitweb_conf() {
>> our \$projectroot = "$(dirname "$fqgitdir")";
>> our \$git_temp = "$fqgitdir/gitweb/tmp";
>> our \$projects_list = \$projectroot;
>> +
>> +$feature{'remote_heads'}{'default'} = [1]
>> EOF
>> }
>
> Thanks.I forgot about trailing semicolon. It should be:
+$feature{'remote_heads'}{'default'} = [1];Show 6 quoted lines
>>> Either solution is fine, but it would require grabbing all the remote >>> heads. The real issue here is, I think understanding what is the >>> purpose of limiting in gitweb. Is it to reduce runtime? is it to >>> reduce clutter on the screen? In the first case, the limiting should >>> be done as early as possible (i.e. during the git call that retrieves >>> the data); in the latter case, is it _really_ needed at all?
[...]
Show 15 quoted lines
>> Regarding gitweb performance, it is quite important to pass limit to >> git-log / git-rev-list needed also for 'summary' view; passing limit >> to git command really matters here. >> >> git_get_heads_list passes '--count='.($limit+1) to git-for-each-ref, >> but I don't think that it improves performance in any measurable way. >> Similar with saving a memory: it is negligible amount. So if we can >> do better at the cost of running git_get_heads_list without a limit, >> I say go for it. > > The gain in performance is, I believe, related to the number of heads > and the number of remotes that are to be enumerated. 11 remotes with a > total of 58 remote branches (the case you mentioned, for example) > might not feel much of a difference between pre- and post-filtering, > but something bigger might start to feel the effect.
Actually I would guess it would depend on what git-for-each-ref does it. I would guess that git-for-each-ref reads in all refs anyway, the limit only matters if format contains fieldnames that need accessing the object,... like e.g. '%(subject)' which git_get_heads_list requests, but git_heads_body doesn't use. Ehh...
> I think the strongest point in favour of post-filtering is that the > feature is intended for use mostly for local repositories anyway.
True.
Show 8 quoted lines
>> Note that the costly part of git_get_heads_list is forking git command, >> so it makes absolutely no sense to run git_get_heads_list once per >> remote instead of doing limiting per-remote in Perl. The former would >> affect performance badly, I can guess. > > That is indeed the reason why I chose to go the single call way, even > though it meant having the limit end up being used somewhat > incorrectly.
I think that single call and post-filtering would be reasonable compromise: reasonable performance (single fork), and correct results.
-- Jakub Narebski Poland