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

[Giuseppe Bilotta <giuseppe.bilotta@gmail.com>]

On Wed, Sep 22, 2010 at 10:34 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> 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:
>
>>>>> 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.

A config option for maximum number of remotes and maximum number of
heads per remotes (in summary) with sane defaults (16/16?) maybe?

>> 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?

That was my idea, yes. It _can_ get a little confusing without CSS3
transitions, due to the 'flash roll down' of the remote heads list. I
will try to cook up a preview sometime this week.

>>> 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".

True. I'll fix that for the next rehash.

-- 
Giuseppe "Oblomov" Bilotta