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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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é
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]