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

From: Shawn Pearce <spearce@spearce.org>
Date: Dec 24, 2006, 02:35 UTC
Message-ID: <20061224023530.GC7443@spearce.org>
In-Reply-To: <7vodpuqtuf.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:

Show 13 quoted lines

> 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.