Re: [PATCH 11/17] Fully activate the sliding window pack access.

[Shawn Pearce <spearce@spearce.org>]

Junio C Hamano <junkio@cox.net> wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> > Hmm. You seem to default the window size to 32MB.
> >
> > Maybe I'm reading that code wrong, but I think that's a bit sad.
> >
> > So I'd argue that if you fall back to read() (or pread) instead of mmap, 
> > the 32MB thing is way too big.
> >
> > So maybe you should make the default depend on NO_MMAP (although it would 
> > seem that the default Makefile makes Cygwin actually default to using mmap 
> > these days, so maybe it's not a big deal).
> 
> I agree that 32MB is too big for emulated mmap().

Yes, I agree too.  I'll submit some additional patches on top of
the existing 17 patch series (which I see is already in 'pu').

> We might want
> to further enhance the new use_pack() API so that the caller can
> say how much it expects to consume, to help pread() based
> emulation avoid reading unnecessary data.

I'm not sure that is worthwhile right now.

The only two callers who know how many bytes they expect is
pack-objects (during delta/whole reuse) and verify-pack (during the
SHA1 check of the packfile itself).  Both are maintenance commands
where preventing a read overshoot in the case of NO_MMAP is probably
not worthwhile, especially as both need to read every byte anyway.

-- 
Shawn.