Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file

["R. Tyler Ballance" <tyler@slide.com>]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

On Wed, 2009-01-07 at 17:01 -0800, Linus Torvalds wrote:
> 
> On Wed, 7 Jan 2009, R. Tyler Ballance wrote:
> > 
> > I was only mentioning OS X with regards to the Samba/NFS red herring,
> > the rest of our operations are on 64-bit Linux machines.
> 
> Ahh, ok. Good. 
> 
> > > I could easily see that if you have a virtual memory size limit of 1.5GB, 
> > > and the pack window size is 1GB, we might have trouble. Because we could 
> > > only keep one such pack window in memory at a time.
> > 
> > The DEFAULT_PACKED_GIT_WINDOW_SIZE in our local builds is 256M, FWIW
> 
> Interesting. So you already had to lower it. However, now that you mention 
> it, and now that I search for your emails about it on the mailing list (I 
> don't normally read the mailing list except very occasionally), I see your 
> patch that does
> 
> 	#define DYNAMIC_WINDOW_SIZE_PERCENTAGE 0.85
> 	...
> 	packed_git_window_size = (unsigned int)(as->rlim_cur * DYNAMIC_WINDOW_SIZE_PERCENTAGE);
> 
> which is actually very bad.
> 
> It's bad for several reasons:
> 
>  - 85% of the virtual address space is actually pessimal.
> 
>    You need space for AT LEAST two full-sized windows, so you need less 
>    than 50%.
> 
>  - the way that variable is used, it _has_ to be a multiple of the page 
>    size. In fact, it needs to be a multiple of _twice_ the page size. So 
>    just doing a random fraction of the rlimit is not correct.

This patch never made it into any of our Git builds because my flight
landed and it wasn't stable enough (and as you pointed out, it sucks ;))



> 
> Setting it in the .gitconfig does it right, though.
> 
> > > If so, then adding a
> > > 
> > > 	[core]
> > > 		packedgitwindowsize = 64M
> > > 
> > > might make a difference. It would certainly be very interesting to hear if 
> > > there's any impact.
> > 
> > I can try this still if you'd like, but it doesn't seem like that'd be
> > the issue since we're already lowering the window size system-wide
> 
> Please do try, at least if your local git changes still match that patch I 
> found, because that patch generates problems.

See my prior reply in "Public repo case!" sent at 4:57PST

-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]