Re: git-index-pack really does suck..

[Nicolas Pitre <nico@cam.org>]

On Tue, 3 Apr 2007, Linus Torvalds wrote:

> 
> 
> On Tue, 3 Apr 2007, Nicolas Pitre wrote:
> >
> > I don't see where that might be.  The only thing that the paranoia check 
> > triggers is:
> > 
> > 	foo = read_sha1_file(blah);
> > 	memcmp(foo with bar);
> > 	free(foo);
> > 
> > So where is that commit ID information actually stored when using 
> > read_sha1_file()?
> 
> I've got the numbers: it uses much more memory when doing even failing 
> lookups, ie:
> 
> 	With --paranoid: 5583 minor pagefaults (21MB)
> 	Without --paranoid: 2957 minor pagefaults (11MB)
> 
> (remember, this was *just* the git pack, not the kernel pack)
> 
> It could be as simple as just the index file itself. That's 11MB for the 
> kernel. Imagine if the index file was 20 times bigger - 200MB of memory 
> paged in with bad access patterns just for unnecessary lookups.

Again that presumes you have to page in the whole index, which should 
not happen when pulling (much smaller packs) and with a better lookup 
algorithm.

> > Not if you consider that it is performed _while_ receiving (and waiting 
> > for) the pack data over the net in the normal case.
> 
> ..which is why I think it makes sense for "pull" to be paranoid. I just 
> don't think it makes sense to be paranoid *all* the time, since it's 
> clearly expensive.

Make it conditionnal on --stdin then.  This covers all cases where we 
really want the secure thing to happen, and the --stdin case already 
perform the atomic rename-and-move thing when the pack is fully indexed.


Nicolas