Re: [PATCH 0/6] address packed-refs speed regressions

["René Scharfe" <l.s.r@web.de>]

Am 05.04.2015 um 20:52 schrieb Jeff King:
> On Sun, Apr 05, 2015 at 03:41:39PM +0200, René Scharfe wrote:
>> I wonder if pluggable reference backends could help here.  Storing refs
>> in a database table indexed by refname should simplify things.
>
> ...this. I think that effort might be better spent on a ref storage
> format that's more efficient, simpler (with respect to subtle races and
> such), and could provide other features (e.g., transactional atomicity).

Such as a DBMS? :-)  Leaving storage details to SQLite or whatever 
sounds attractive to me because I'm lazy.

> The big plus side of packed-refs improvements is that they "just work"
> without worrying about compatibility issues. But ref storage is local,
> so I'm not sure how big a deal that is in practice.

Adding a dependency is a big step, admittedly, so native improvements 
might be a better fit.  There's a chance that we'd run into issues 
already solved by specialized database engines long ago, though.

>> Short-term, can we avoid the getc()/strbuf_grow() dance e.g. by mapping
>> the packed refs file?  What numbers do you get with the following patch?
>
> It's about 9% faster than my series + the fgets optimization I posted
> (or about 25% than using getc).  Which is certainly nice, but I was
> really hoping to just make strbuf_getline faster for all callers, rather
> than introducing special code for one call-site.  Certainly we could
> generalize the technique (i.e., a struct with the mmap data), but then I
> feel we are somewhat reinventing stdio. Which is maybe a good thing,
> because stdio has a lot of rough edges (as seen here), but it does feel
> a bit like NIH syndrome.

Forgot to say: I like your changes.  But if strbuf_getline can only be 
made fast enough beyond that by duplicating stdio buffering then I feel 
it's better to take a different way.  E.g. dropping the requirement to 
handle NUL chars and basing it on fgets as Junio suggested in his reply 
to patch 3 sounds good.

In any case, the packed refs file seems special enough to receive 
special treatment.  Using mmap would make the most sense if we could 
also avoid copying lines to a strbuf for parsing, though.

René