Re: [PATCH 7/7] gitweb: group remote heads

From: Jakub Narebski <jnareb@gmail.com>
Date: Sep 22, 2010, 08:34 UTC
Message-ID: <201009221034.10958.jnareb@gmail.com>
In-Reply-To: <AANLkTi=20gzw8DOuT=NWRqbt86AyJDEPxoo-V9A3Rvim@mail.gmail.com>

On Mon, Sep 20, 2010, Giuseppe Bilotta wrote:

> On Mon, Sep 20, 2010 at 10:59 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> Giuseppe Bilotta wrote:
>>> On Mon, Sep 20, 2010 at 1:02 AM, Jakub Narebski <jnareb@gmail.com> wrote:

Show 21 quoted lines

>>>> The solution (1) i.e. limiting number of remote heads per remote, with
>>>> or without limiting number of remotes behaves, as you wrote, most
>>>> similarly to other components of 'summary' view.  On the other hand
>>>> with large number of remotes, and large number of remote heads in those
>>>> remotes it might be too large for a *summary* view.
>>>
>>> So you maintain that limiting the amount of data in summary view
>>> should be primary wrt to limiting the amount of time?
>>
>> Well, what really affect gitweb performance is calling git commands, both
>> because of fork overhead, and because it means disk access (and gitweb
>> performance from what I have heard is affected mainly by IO, and not CPU).
>> With grouping (displaying remotes) the difference between displaying
>> remote-tracking branches (or information from them) and not displaying
>> them is an argument to git-for-each-ref.  So I don't think it would
>> affect performance much.
> 
> Getting the list of remote branches is, I would say, the most
> IO-intensive operation. I'm not sure how much I/O it would do though,
> even with a large number of remotes and heads. So maybe always gather
> all the information is the way to go.

Well, that depends on the number of remote-tracking branches, and whether
those refs are packed (see git-pack-refs).  But I agree that getting list
of all remote branches might be I/O hit... though I am not sure if it
would be large enough to visibly affect gitweb performance.
 
By the way, besides the solution described below (list only remotes if
there are many of them), we could make 'remote_heads' be not simply
boolean, but to be multiple-choice feature, configuring how 'remotes'
view and remotes section in 'summary' page looks like.  But that might
be overengineering.

Show 13 quoted lines

>>> If time spent processing is not an issue, we can retrieve the number
>>> of heads for each remote and display that, for example. Or even play
>>> with some more dynamic stuff like making each group collapsible,
>>> starting with it collapsed and then display the content when the user
>>> hovers it with the mouse, for example.
>>
>> The dynamic stuff is IMHO a good idea... provided we can either do it
>> without JavaScript, or we can ensure that browser supports JavaScript
>> (see current hack used for turning 'blame' into 'blame_incremental'
>> view in gitweb).
> 
> What I had in mind was something that is very easy to implement with
> CSS only.

Do I understand correctly that you would utilize ':hover' pseudoclass
and 'display: none;' style?  How ergonomic would this solution be?

Show 11 quoted lines

>> Yet another solution would be to display only abbreviated list of remotes
>> if its more of them than some threshold, and list remotes with abbreviated
>> list of remote-tracking branches if there are only a few remotes.
> 
> So something like this:
> 
> (1) if there are more than N remotes, only show N remote _names_ (no heads)
> (2) if there are no more than N remotes, show all remote names, each
> with no more than M heads
> 
> (with N and M to be decided, e.g. the usual 16)

Right.

Show 16 quoted lines

>>> Yes, this is something I have to take into consideration. Skip
>>> displaying them is probably the best idea (unless we have other ways
>>> to gather information about them).
>>
>> Right.
> 
> For this, it would be nice to have `git remote show`, but even if I
> sent a patch to this effect gitweb should probably be left able to
> cope with older git versions not supporting it ...
> 
>> P.S. It is not necessary for this series, but I think we should think
>> about "single remote" view... also because your code currently links
>> to such views, which do not exist yet (remotes/<remote> in path_info:
>> how it would be represented in CGI query format?).
> 
> Maybe pass the remote name as head parameter?

In the meantime, while we don't have 'remote' view for a single remote (something like "git remote show"), it would be good if the header for individual remotes didn't lead to "'remotes/<remote>' action".

-- 
Jakub Narebski
Poland