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

[Jakub Narebski <jnareb@gmail.com>]

Giuseppe Bilotta wrote:
> On Mon, Sep 20, 2010 at 1:02 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> >
> > When thinking about how display information about remotes and
> > remote-tracking branches in 'summary' view, we have to consider that
> > this view is meant to be what it says: a *summary*.  That is why both
> > the 'heads' and the 'tags' section displays only 15 items.
> >
> > Without grouping remote heads the natural solution was to simply repeat
> > what we used for 'heads' subsection, namely limit displaying to 15
> > remote-tracking branches in 'remotes' subsection of the 'summary' view.
> >
> > With grouping it is more complicated.  We can limit number of remote
> > head *per remote*, we can limit number of remotes... but what makes
> > IMVHO least sense is limit number of remote heads *then* do grouping.
> 
> This is something I absolutely agree with, BTW. It is the way it's
> implemented currently because it was the quickest way to ship a
> prototype out for discussion.

All right.  Prototyping is good.

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

> > The solution (3) i.e. displaying only list of remotes (perhaps limited
> > to 15 remotes) is simple and fast to render.  On the other hand it offers
> > least information and might be too little in the case of single remote.
> 
> 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).

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.
 
> > > If we go with option 3, it does make sense to get all remote names and
> > > all remote branches at once, and thus to make the git_get_remotes()
> > > call do all of the work.
> >
> > Not if subroutine is called git_get_remotes(), IMVHO.  Perhaps
> > git_group_remotes()?
> 
> That I can do.

> > If remote doesn't have remote-tracking branches associated with it
> > (e.g. push-only remote, or remote predating separate remotes layout)
> > the value would be undef for given remote as key in hash.
> 
> 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.

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

-- 
Jakub Narebski
Poland