Subject: Re: [PATCH 1/5] gitweb: Option for filling only specified info in fill_project_list_info

From: Jakub Narebski <[email-masked]>
Date: Fri, 10 Feb 2012 22:30:12 +0100

Junio C Hamano wrote:
> Jakub Narebski <[email-masked]> writes:
> 
> > If @fill_only is empty, it means for fill_project_list_info to fill
> > all the data, if it is not empty it means that those fields needs to
> > be filled.
> 
> That is exactly what I am questioning.  Do you need "I need all these
> fields to be present" and "I will fill these other fields" (which is what
> @fill_only is about, no?) that is different from the former?

"I need all these fields" is a property of callsite.  fill_project_list_info()
will be called more than one time in 2/5 to incrementally complete project
info, see below.
 
"I will fill those fields" is a property of piece of code protected by
a conditional inside fill_project_list_info().

> > The code of fill_project_list_info goes like this
> >
> >   if (do we need to fill 'age' or 'age_string'?) {
> >     fill 'age' and 'age_string'
> >   }
> >   if (do we need to fill 'desc_long' or 'descr'?) {
> >     fill 'descr_long' and 'descr'
> >   }
> >   if (we are interested in 'ctags' &&
> >       do we need to fill 'ctags'?) {
> >     fill 'ctags'
> >   }
> >   ...
> 
> Exactly.  Why do you need @fill_only at all?  If you are interested in
> ctags and you want to make sure ctags is available, the question you want
> to ask the helper function is "Does the project structure already have
> ctags field?".  Why does the helper function needs to know anything else?

It is to support incremental filling of project info.  The code is to
go like this:

  create
  filter
  fill part
  filter
  fill rest

We need @fill_only for the "fill part".  As filling project info is
potentially expensive (especially the 'age' field), doing it on narrowed
(filtered) list of project is a performance win.  That is what 2/5 is
about.

-- 
Jakub Narebski
Poland