Re: [PATCH] avoid possible overflow in delta size filtering computation

From: Kjetil Barvik <barvik@broadpark.no>
Date: Mar 26, 2009, 07:18 UTC
Message-ID: <86y6usah1p.fsf@broadpark.no>
In-Reply-To: <alpine.LFD.2.00.0903251514360.26337@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

Show 27 quoted lines

> On Wed, 25 Mar 2009, Kjetil Barvik wrote:
>
>> Nicolas Pitre <nico@cam.org> writes:
>> 
>> > On Wed, 25 Mar 2009, Kjetil Barvik wrote:
>> >
>> >>   So, it seems that this patch almost fixed the issue.  But notice that
>> >>   the pack file was 10 bytes larger for the --depth=95000 case.
>> >> 
>> >>   I made a small perl script to compare the output from 'git verify-pack
>> >>   -v' of the 2 idx/pack files, and found the following difference(1)
>> >>   (first line from --depth=20000 case, second from --depth=95000):
>> >> 
>> >>   fe0a6f3e971373590714dbafd087b235ea60ac00  tree   9  19  18921247  731  96a3ec5789504e6d0f90c99fb1937af1ebd58e2d
>> >>   fe0a6f3e971373590714dbafd087b235ea60ac00  tree  20  29  18921247  730  12e560f7fb28558b15e3a2008fba860f9a4b2222
>> >
>> > OK.  Apparently, a different base object for that one delta was chosen 
>> > between those two runs.
>> >
>> > Is your machine SMP?
>> 
>>   kjetil ~$ uname -a
>>   Linux localhost 2.6.28.4 #26 SMP PREEMPT Tue Feb 10 17:07:14 CET 2009
>>   i686 Intel(R) Core(TM)2 CPU T7200 @ 2.00GHz GenuineIntel GNU/Linux
>
> Here you go.  If you want a perfectly deterministic repacking, you'll 
> have to force the pack.threads config option to 1.

  OK, have rerun the test again with pack.config set to 1, and got the
  exact same result(1) as above this time also!  :-)

  And I think I can explain the reason for the same result: When the
  --window and/or --depth value(s) is larger than (half?) the value of
  possible number of objects (98438 in this time), I think that the
  thread logic finds out that it can only run one thread (there is not
  room/objects enough for 2 or more threads).

  This also explains what I see when I run the repack command (without
  the pack.config option), only 1 git process is running on 1 of the
  CPU's from the start, and the other is idle.

~~

  Give me a hint if you want some debug info from the 2 pack/idx files.

  -- kjetil

  1) The output from the 2 'git repack' commands did not show the
     "running delta with 2 threads" (or something similar) this time.
     I guess this is the sign that "pack.threads = 1" is working.