Git development
 help / color / mirror / Atom feed
* Re: 1.3.0 creating bigger packs than 1.2.3
From: Shawn Pearce @ 2006-04-20 15:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604200745550.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> wrote:
> 
> On Thu, 20 Apr 2006, Shawn Pearce wrote:
> 
> > Apparently I have created a repository which v1.2.3 packs about 50%
> > smaller than 'next' does:
> > 
> >   v1.2.3 (tag):
> >    60M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> > 
> >   1.2.3.gf3a4 (an older 'next'):
> >   128M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> > 
> >   1.3.0.rc4.g8060 (a fairly recent 'next'):
> >   118M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> > 
> > Repeated packing with 1.3.0.rc4.g8060 doesn't seem to change the
> > size of the pack file, its pretty consistent at 118M.
> 
> First try "git repack -a -d f", where the "-f" is the magic one.
> 
> Without the -f, git repack will re-use old pack information, which is much 
> much faster, but not as space-efficient.
> 
> If that doesn't help, it might be time to look at the actual repo, but try 
> that first.

So with 1.3.0.g56c1 "git repack -a -d -f" did worse:

  Total 46391, written 46391 (delta 6649), reused 39742 (delta 0)
  129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

I just tried -f on v1.2.3 and it did slightly better then before:

  Total 46391, written 46391 (delta 6847), reused 38012 (delta 0)
   59M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

-- 
Shawn.

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Linus Torvalds @ 2006-04-20 16:07 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060420150315.GB31198@spearce.org>



On Thu, 20 Apr 2006, Shawn Pearce wrote:
> 
> So with 1.3.0.g56c1 "git repack -a -d -f" did worse:
> 
>   Total 46391, written 46391 (delta 6649), reused 39742 (delta 0)
>   129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> 
> I just tried -f on v1.2.3 and it did slightly better then before:
> 
>   Total 46391, written 46391 (delta 6847), reused 38012 (delta 0)
>    59M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

Interesting. The bigger packs do generate fewer deltas, but they don't 
seem to be _that_ much fewer. And the deltas themselves certainly 
shouldn't be bigger.

It almost sounds like there's a problem with choosing what to delta 
against, not necessarily a delta algorithm problem. Although that sounds a 
bit strange, because I wouldn't have thought we actually changed the 
packing algorithm noticeably since 1.2.3.

Hmm. Doing "gitk v1.2.3.. -- pack-objects.c" shows that I was wrong. Junio 
did the "hash basename and direname a bit differently" thing, which would 
appear to change the "find objects to delta against" a lot. That could be 
it. 

You could try to revert that change:

	git revert eeef7135fed9b8784627c4c96e125241c06c65e1

which needs a trivial manual fixup (remove the conflict entirely: 
everything between the "<<<<" and ">>>>>" lines should go), and see if 
that's it.

You can also try to see if

	git repack -a -d -f --window=50

makes for a better pack (at the cost of a much slower repack). It makes 
git try more objects to delta against, and can thus hide a bad sort order.

Junio, any other suggestions?

		Linus

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Nicolas Pitre @ 2006-04-20 16:09 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060420133640.GA31198@spearce.org>

On Thu, 20 Apr 2006, Shawn Pearce wrote:

> Given that disk is pretty cheap these days I'm not concerned about
> the 2x increase but thought I'd let folks know that the packing
> improvements in 1.3.0 seem to have taken a small step backwards
> with regards to this particular dataset.

2x is a huge regression.

> I can make the repository available if somebody wants to look at it.

Please.


Nicolas

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Shawn Pearce @ 2006-04-20 16:43 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604200857460.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> wrote:
> 
> 
> On Thu, 20 Apr 2006, Shawn Pearce wrote:
> > 
> > So with 1.3.0.g56c1 "git repack -a -d -f" did worse:
> > 
> >   Total 46391, written 46391 (delta 6649), reused 39742 (delta 0)
> >   129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> > 
> > I just tried -f on v1.2.3 and it did slightly better then before:
> > 
> >   Total 46391, written 46391 (delta 6847), reused 38012 (delta 0)
> >    59M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

Oddly enough repacking the v1.2.3 pack using 1.3.0.g56c1 created an
even smaller pack ("git-repack -a -d"):

  Total 46391, written 46391 (delta 8253), reused 44985 (delta 6847)
   49M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

and repacking again with "git-repack -a -d" chopped another 1M:

  Total 46391, written 46391 (delta 8258), reused 46386 (delta 8253)
   48M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pac
  
but then adding -f definately gives us the 2x explosion again:

  Total 46391, written 46391 (delta 6649), reused 37894 (delta 0)
  129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

> Interesting. The bigger packs do generate fewer deltas, but they don't 
> seem to be _that_ much fewer. And the deltas themselves certainly 
> shouldn't be bigger.
> 
> It almost sounds like there's a problem with choosing what to delta 
> against, not necessarily a delta algorithm problem. Although that sounds a 
> bit strange, because I wouldn't have thought we actually changed the 
> packing algorithm noticeably since 1.2.3.
> 
> Hmm. Doing "gitk v1.2.3.. -- pack-objects.c" shows that I was wrong. Junio 
> did the "hash basename and direname a bit differently" thing, which would 
> appear to change the "find objects to delta against" a lot. That could be 
> it. 
> 
> You could try to revert that change:
> 
> 	git revert eeef7135fed9b8784627c4c96e125241c06c65e1
> 
> which needs a trivial manual fixup (remove the conflict entirely: 
> everything between the "<<<<" and ">>>>>" lines should go), and see if 
> that's it.

Whoa.  I did that revert and fixup on top of 'next'.  The pack
from "git-repack -a -d -f" is now even larger due to even less
delta reuse:

  Total 46391, written 46391 (delta 5148), reused 39565 (delta 0)
  171M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

> You can also try to see if
> 
> 	git repack -a -d -f --window=50
> 
> makes for a better pack (at the cost of a much slower repack). It makes 
> git try more objects to delta against, and can thus hide a bad sort order.

With --window=50 on 'next' (without the revert'):

  Total 46391, written 46391 (delta 6666), reused 39723 (delta 0)
  129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

For added measure I tried --window=100 and 500 with pretty much
the same result (slightly higher delta but still a 129M pack).

-- 
Shawn.

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Zack Brown @ 2006-04-20 16:49 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060419144827.GX27631@pasky.or.cz>

Hi Petr,

On Wed, Apr 19, 2006 at 04:48:27PM +0200, Petr Baudis wrote:
>   Hi,
> 
> Dear diary, on Wed, Apr 19, 2006 at 04:21:31PM CEST, I got a letter
> where Zack Brown <zbrown@tumblerings.org> said that...
> > On Wed, Apr 19, 2006 at 11:49:16AM +0200, Petr Baudis wrote:
> > > Dear diary, on Wed, Apr 19, 2006 at 07:36:40AM CEST, I got a letter
> > > where Zack Brown <zbrown@tumblerings.org> said that...
> > > > When I do something like
> > > > cg-clone rsync://rsync.kernel.org/pub/scm/linux/kernel/git/torvalds/git.git
> > > > 
> > > > The first few lines of output are:
> > > > 
> > > > defaulting to local storage area
> > > > warning: templates not found /home/zbrown/share/git-core/templates/
> > > > /home/zbrown/git/cogito/cg-clone: line 137: .git/info/cg-fetch-earlydie: No such file or directory
> > > > /home/zbrown/git/cogito/cg-clone: line 148: .git/info/cg-fetch-initial: No such file or directory
> > > 
> > > Could you please list the contents of the .git subdirectory? It seems
> > > that git-init-db did not create the .git/info subdirectory.
> > 
> > 07:19:57 [zbrown] ~/git/trees/tmp/git/.git$ ls -F
> > total 28
> > 4 HEAD  4 branches/  4 config  4 index  4 info/  4 objects/  4 refs/
> 
>   hmm, could you please do this just after running git-init-db in an
> empty directory? I just realized cg-fetch will mkdir -p the .git/info/
> directory.

You're right, the "info" directory is not there if I just run git-init-db in an
empty directory.

> error. If the .git/info/ directory is not there after git-init-db,
> either it is somehow broken in git-1.3.0, or it belongs to a much older
> git version.

I just downloaded the latest versions of git and cogito from kernel.org:
cogito-0.17.2 and git-1.3.0; put their directories in my path, and ran "make" on
both of them. There's no other version in my path.

I see the same behavior: git-init-db does not create the .git/info directory.

> 
> > 07:18:38 [zbrown] ~$ which git-init-db
> > /home/zbrown/git/git//git-init-db
> > 07:18:52 [zbrown] ~$ which git        
> > /home/zbrown/git/git//git
> 
>   It might be a good idea to compare the ctimes.

09:46:55 [zbrown] ~/git/trees$ "ls" -ltc `which git; which git-init-db`
-rwxrwxr-x 2 zbrown zbrown 452312 Apr 20 09:44 /home/zbrown/git/git//git
-rwxrwxr-x 1 zbrown zbrown 235282 Apr 20 09:43 /home/zbrown/git/git//git-init-db
09:47:29 [zbrown] ~/git/trees$ 

Be well,
Zack

> 
> -- 
> 				Petr "Pasky" Baudis
> Stuff: http://pasky.or.cz/
> Right now I am having amnesia and deja-vu at the same time.  I think
> I have forgotten this before.

-- 
Zack Brown

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Linus Torvalds @ 2006-04-20 17:03 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Git Mailing List, Nicolas Pitre
In-Reply-To: <20060420164351.GB31738@spearce.org>



On Thu, 20 Apr 2006, Shawn Pearce wrote:
> 
> Oddly enough repacking the v1.2.3 pack using 1.3.0.g56c1 created an
> even smaller pack ("git-repack -a -d"):

That's "normal". Repacking without -f will always pack _more_, never less. 
So a different packing algorithm can only improve (of course, usually not 
by a huge margin, and it quickly diminishes).

> but then adding -f definately gives us the 2x explosion again:
> 
>   Total 46391, written 46391 (delta 6649), reused 37894 (delta 0)
>   129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

Right. Doing the -f will discard any old packing info, so if the new 
packing algorithm has problems (and it obviously does), then using -f will 
show them.

> > You could try to revert that change:
> > 
> > 	git revert eeef7135fed9b8784627c4c96e125241c06c65e1
> 
> Whoa.  I did that revert and fixup on top of 'next'.  The pack
> from "git-repack -a -d -f" is now even larger due to even less
> delta reuse:

Ok, so that wasn't it, and the new sort order is superior.

That means that it probably _is_ the delta changes themselves (probably 
commit c13c6bf7 "diff-delta: bound hash list length to avoid O(m*n) 
behavior". You can try

	git revert c13c6bf7

to see if that's it. Although Nico already showed interest, and if you 
make the archive available to him, he's sure to figure it out.

> With --window=50 on 'next' (without the revert'):
> 
>   Total 46391, written 46391 (delta 6666), reused 39723 (delta 0)
>   129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack

Yeah, that didn't do much. Slightly more deltas than without, but not a 
lot, and it didn't matter much size-wise.

You can try "--depth=50" (slogan: more "hot delta on delta action"), but 
it's looking less and less like a delta selection issue, and more and more 
like the deltas themselves are deproved.

			Linus

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Junio C Hamano @ 2006-04-20 17:24 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604200954440.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> You can try "--depth=50" (slogan: more "hot delta on delta action"), but 
> it's looking less and less like a delta selection issue, and more and more 
> like the deltas themselves are deproved.

I do not think I have time to look into this today until late
night, but one thing I noticed is that trying to delta more
things sometimes tend to produce bigger result X-<.  

At the end of pack-objects.c::find_deltas(), there is a code
that is commented out, which is remnant from a failed
experiment.  What it tried to do was to avoid placing an object
whose delta depth is already window-size back into the
candidates list, which means the next object gets compared with
one object more than otherwise would be (the extra one being the
oldest one in the window -- which might not produce better delta
than the maxed out one, but the delta with the maxed out one
would not be used anyway).  The result was noticeably worse
overall packsize with more deltified objects.

We might be better off favoring compressed undeltified
representation over deltified representation a bit more
aggressively.  Currently we allow delta as big as half the
uncompressed size minus 20-byte overhead or something like that;
tweaking that limit might show improvements.

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Shawn Pearce @ 2006-04-20 17:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Nicolas Pitre
In-Reply-To: <Pine.LNX.4.64.0604200954440.3701@g5.osdl.org>

I just spent some time bisecting this issue and it looks like the
following change by Junio may be the culprit:

  commit 1d6b38cc76c348e2477506ca9759fc241e3d0d46
  Author: Junio C Hamano <junkio@cox.net>
  Date:   Wed Feb 22 22:10:24 2006 -0800
  
      pack-objects: use full pathname to help hashing with "thin" pack.
      
      This uses the same hashing algorithm to the "preferred base
      tree" objects and the incoming pathnames, to group the same
      files from different revs together, while spreading files with
      the same basename in different directories.
      
      Signed-off-by: Junio C Hamano <junkio@cox.net>
  
  :100644 100644 af3bdf5d358b8a47ed23bcb7e9721e956eb59d60 3a16b7e4ce25ec05c64817dfd92dd9d517ab9dd3 M      pack-objects.c


Linus Torvalds <torvalds@osdl.org> wrote:
> 
> 
> On Thu, 20 Apr 2006, Shawn Pearce wrote:
> > 
> > Oddly enough repacking the v1.2.3 pack using 1.3.0.g56c1 created an
> > even smaller pack ("git-repack -a -d"):
> 
> That's "normal". Repacking without -f will always pack _more_, never less. 
> So a different packing algorithm can only improve (of course, usually not 
> by a huge margin, and it quickly diminishes).
> 
> > but then adding -f definately gives us the 2x explosion again:
> > 
> >   Total 46391, written 46391 (delta 6649), reused 37894 (delta 0)
> >   129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> 
> Right. Doing the -f will discard any old packing info, so if the new 
> packing algorithm has problems (and it obviously does), then using -f will 
> show them.
> 
> > > You could try to revert that change:
> > > 
> > > 	git revert eeef7135fed9b8784627c4c96e125241c06c65e1
> > 
> > Whoa.  I did that revert and fixup on top of 'next'.  The pack
> > from "git-repack -a -d -f" is now even larger due to even less
> > delta reuse:
> 
> Ok, so that wasn't it, and the new sort order is superior.
> 
> That means that it probably _is_ the delta changes themselves (probably 
> commit c13c6bf7 "diff-delta: bound hash list length to avoid O(m*n) 
> behavior". You can try
> 
> 	git revert c13c6bf7
> 
> to see if that's it. Although Nico already showed interest, and if you 
> make the archive available to him, he's sure to figure it out.
> 
> > With --window=50 on 'next' (without the revert'):
> > 
> >   Total 46391, written 46391 (delta 6666), reused 39723 (delta 0)
> >   129M pack-7f766f5af5547554bacb28c0294bd562589dc5e7.pack
> 
> Yeah, that didn't do much. Slightly more deltas than without, but not a 
> lot, and it didn't matter much size-wise.
> 
> You can try "--depth=50" (slogan: more "hot delta on delta action"), but 
> it's looking less and less like a delta selection issue, and more and more 
> like the deltas themselves are deproved.
> 
> 			Linus

-- 
Shawn.

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Junio C Hamano @ 2006-04-20 17:36 UTC (permalink / raw)
  To: Zack Brown; +Cc: git
In-Reply-To: <20060420164908.GA540@tumblerings.org>

Zack Brown <zbrown@tumblerings.org> writes:

> I just downloaded the latest versions of git and cogito from kernel.org:
> cogito-0.17.2 and git-1.3.0; put their directories in my path, and ran "make" on
> both of them. There's no other version in my path.

Earlier, you were having this symptom:

>> What do these command say?
>> 
>> 	$ git --exec-path
>> 	$ ls -l "`git --exec-path`/git-clone"
>
> 22:07:05 [zbrown] ~$ git --exec-path
> /home/zbrown/bin
> 07:10:34 [zbrown] ~$ ls -l "`git --exec-path`/git-clone"
> ls: /home/zbrown/bin/git-clone: No such file or directory
>
> Does that mean it's looking in /home/zbrown/bin for the git binaries?

If that is the case, you did not just (quote) "and ran "make"".

You must have run "make frotz=xyzzy target", but you did not mention
what frotz, xyzzy and target were.

> 09:46:55 [zbrown] ~/git/trees$ "ls" -ltc `which git; which git-init-db`
> -rwxrwxr-x 2 zbrown zbrown 452312 Apr 20 09:44 /home/zbrown/git/git//git
> -rwxrwxr-x 1 zbrown zbrown 235282 Apr 20 09:43 /home/zbrown/git/git//git-init-db

So you are doing

	make bindir=$HOME/git/git/ install

as the last step of your installation.  I _strongly_ suspect
your breakage is caused because you did a make with different
configuration before that.  That is, if you do this, that is
consistent with the symptom:

	make
	make bindir=$HOME/git/git/ gitexecdir=$HOME/git/git/ install

It probably would help if you did this:

	make clean
	make bindir=$HOME/git/git gitexecdir=$HOME/git/git/
	make bindir=$HOME/git/git gitexecdir=$HOME/git/git/ install

As I said in a previous message, the first paragraph in INSTALL
file explains this.

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Nicolas Pitre @ 2006-04-20 17:41 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Shawn Pearce, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0604200954440.3701@g5.osdl.org>

On Thu, 20 Apr 2006, Linus Torvalds wrote:

> That means that it probably _is_ the delta changes themselves (probably 
> commit c13c6bf7 "diff-delta: bound hash list length to avoid O(m*n) 
> behavior". You can try
> 
> 	git revert c13c6bf7
> 
> to see if that's it. Although Nico already showed interest, and if you 
> make the archive available to him, he's sure to figure it out.

It is not that.  With that code disabled there is still a 2x pack size.

Substituting diff-delta.c from the version in 1.2.3 doesn't solve the 
issue either.


Nicolas

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Nicolas Pitre @ 2006-04-20 17:54 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20060420173131.GF31738@spearce.org>

On Thu, 20 Apr 2006, Shawn Pearce wrote:

> I just spent some time bisecting this issue and it looks like the
> following change by Junio may be the culprit:
> 
>   commit 1d6b38cc76c348e2477506ca9759fc241e3d0d46
>   Author: Junio C Hamano <junkio@cox.net>
>   Date:   Wed Feb 22 22:10:24 2006 -0800
>   
>       pack-objects: use full pathname to help hashing with "thin" pack.
>       
>       This uses the same hashing algorithm to the "preferred base
>       tree" objects and the incoming pathnames, to group the same
>       files from different revs together, while spreading files with
>       the same basename in different directories.
>       
>       Signed-off-by: Junio C Hamano <junkio@cox.net>
>   
>   :100644 100644 af3bdf5d358b8a47ed23bcb7e9721e956eb59d60 3a16b7e4ce25ec05c64817dfd92dd9d517ab9dd3 M      pack-objects.c

Hmmm... This one is for Junio to fix I'd say.  Not sure what it does.


Nicolas

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Shawn Pearce @ 2006-04-20 17:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Nicolas Pitre
In-Reply-To: <Pine.LNX.4.64.0604200954440.3701@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> wrote:
> Ok, so that wasn't it, and the new sort order is superior.
> 
> That means that it probably _is_ the delta changes themselves (probably 
> commit c13c6bf7 "diff-delta: bound hash list length to avoid O(m*n) 
> behavior". You can try
> 
> 	git revert c13c6bf7

No effect.
 
> to see if that's it. Although Nico already showed interest, and if you 
> make the archive available to him, he's sure to figure it out.

I sent the URL privately to Nico as I did not want the repository
to be publically available before next Tuesday.

> You can try "--depth=50" (slogan: more "hot delta on delta action"), but 
> it's looking less and less like a delta selection issue, and more and more 
> like the deltas themselves are deproved.

No effect at either 50 or 100.

The more that I think about it the more it seems possible that the
pathname hashing is what may be causing the problem.  Not only did
bisect point to 1d6b38cc76c348e2477506ca9759fc241e3d0d46 but the
directory which contains the bulk of the space has many files with
the same name located in different directories:

	results/MT/Math/10000/0-11-AdjLite.deg
	results/MT/Math/10000/0-12-AdjLite.deg
	...
	results/MT/Math/30000/2-11-AdjLite.deg
	results/MT/Math/30000/2-12-AdjLite.deg
	...
	results/Rand48/Math/10000/2-11-AdjLite.deg
	results/Rand48/Math/10000/2-12-AdjLite.deg
	...
	results/Rand48/Math/30000/2-11-AdjLite.deg
	results/Rand48/Math/30000/2-12-AdjLite.deg
	...

For example the name '0-11-AdjLite.deg' occurs in 63 directories and
none of those occurrances are likely to delta against one another
very well.  Also most of these files only have 1 or 2 revisions,
so there is very little per-file history.

-- 
Shawn.

^ permalink raw reply

* Re: n-heads and patch dependency chains
From: Jon Loeliger @ 2006-04-20 18:08 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Jakub Narebski, Git List
In-Reply-To: <44325CDB.2000101@op5.se>

On Tue, 2006-04-04 at 06:47, Andreas Ericsson wrote:

> No, I mean that this would commit both to the testing branch (being the 
> result of several merged topic-branches) and to the topic-branch merged 
> in. Commit as in regular commit, with a commit-message and a patch. The 
> resulting repository would be the exact same as if the change was 
> committed only to the topic-branch and then cherry-picked on to the 
> testing-branch.

I am your number one fan!  If I finish reading these 600+
messages, will I find out you have already implemented it,
it's committed, and you just need me to test it now? :-)

jdl

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Nicolas Pitre @ 2006-04-20 18:24 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20060420175554.GH31738@spearce.org>

On Thu, 20 Apr 2006, Shawn Pearce wrote:

> The more that I think about it the more it seems possible that the
> pathname hashing is what may be causing the problem.  Not only did
> bisect point to 1d6b38cc76c348e2477506ca9759fc241e3d0d46 but the
> directory which contains the bulk of the space has many files with
> the same name located in different directories:
[...]

But the bad commit according to your bisection talks about "thin" packs 
which are not involved in your case.  So something looks fishy with that 
commit which should not have touched path hashing in the non-thin pack 
case...  I think...


Nicolas

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Junio C Hamano @ 2006-04-20 18:49 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604201414490.2215@localhost.localdomain>

Nicolas Pitre <nico@cam.org> writes:

> On Thu, 20 Apr 2006, Shawn Pearce wrote:
>
>> The more that I think about it the more it seems possible that the
>> pathname hashing is what may be causing the problem.  Not only did
>> bisect point to 1d6b38cc76c348e2477506ca9759fc241e3d0d46 but the
>> directory which contains the bulk of the space has many files with
>> the same name located in different directories:
> [...]
>
> But the bad commit according to your bisection talks about "thin" packs 
> which are not involved in your case.  So something looks fishy with that 
> commit which should not have touched path hashing in the non-thin pack 
> case...  I think...

I think this explains it.  The new code hashes full-path, but
places bins for the paths with the same basename next to each
other, so before Makefile and doc/Makefile and t/Makefile were
all in the same bin, but now they are in three different bins
next to each other.

I originally thought, with one single notable exception of
Makefile, having the identically named file in many different
directories is not common nor sane, and the new code favors to
delta with the exact same path for deeper history over wasting
delta window for making delta with objects with the same name in
different places in more recent history.  I think I benched this
with kernel repository (git.git was too small for that).

But I suspect we have a built-in "we sort bigger to smaller, and
we cut off when we switch bins" somewhere in find_delta() loop,
which I do not recall touching when I did that change, so that
may be interfering and preventing 0-11-AdjLite.deg from all over
the place to delta against each other.

^ permalink raw reply

* Re: n-heads and patch dependency chains
From: Junio C Hamano @ 2006-04-20 18:55 UTC (permalink / raw)
  To: git
In-Reply-To: <1145556505.5314.149.camel@cashmere.sps.mot.com>

Jon Loeliger <jdl@freescale.com> writes:

> On Tue, 2006-04-04 at 06:47, Andreas Ericsson wrote:
>
>> No, I mean that this would commit both to the testing branch (being the 
>> result of several merged topic-branches) and to the topic-branch merged 
>> in. Commit as in regular commit, with a commit-message and a patch. The 
>> resulting repository would be the exact same as if the change was 
>> committed only to the topic-branch and then cherry-picked on to the 
>> testing-branch.

To be consistent, I think the result should be "as if the change
was commited only to the topic-branch and then the topic-branch
was *merged* into the testing-branch", since you start your
testing branch as "being the result of several merged topic-branches".

I do that (manually) all the time, with:

	$ git checkout next
        $ hack hack hack

        $ git checkout -m one/topic
        $ git commit -o this-path that-path
        $ git checkout next
        $ git pull . one/topic

Giving a short-hand for the last four-command sequence would
certainly be nice.

> I am your number one fan!  If I finish reading these 600+
> messages, will I find out you have already implemented it,
> it's committed, and you just need me to test it now? :-)

Likewise... ;-)

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Zack Brown @ 2006-04-20 20:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejzsywrq.fsf@assigned-by-dhcp.cox.net>

On Thu, Apr 20, 2006 at 10:36:25AM -0700, Junio C Hamano wrote:
> Zack Brown <zbrown@tumblerings.org> writes:
> 
> > I just downloaded the latest versions of git and cogito from kernel.org:
> > cogito-0.17.2 and git-1.3.0; put their directories in my path, and ran "make" on
> > both of them. There's no other version in my path.
> 
> Earlier, you were having this symptom:
> 
> >> What do these command say?
> >> 
> >> 	$ git --exec-path
> >> 	$ ls -l "`git --exec-path`/git-clone"
> >
> > 22:07:05 [zbrown] ~$ git --exec-path
> > /home/zbrown/bin
> > 07:10:34 [zbrown] ~$ ls -l "`git --exec-path`/git-clone"
> > ls: /home/zbrown/bin/git-clone: No such file or directory
> >
> > Does that mean it's looking in /home/zbrown/bin for the git binaries?
> 
> If that is the case, you did not just (quote) "and ran "make"".
> 
> You must have run "make frotz=xyzzy target", but you did not mention
> what frotz, xyzzy and target were.

Not true. I went into the git source directory, and ran "make". Nothing more.

I've been doing that for a long time, whenever I sync with the repository. I
didn't know the installation instructions had changed.

> It probably would help if you did this:
> 
> 	make clean
> 	make bindir=$HOME/git/git gitexecdir=$HOME/git/git/
> 	make bindir=$HOME/git/git gitexecdir=$HOME/git/git/ install

OK, I did this. The first 2 commands worked fine. The third complained of
duplicate files, and exited with an error. Maybe because the source tree is also
$HOME/git/git

I then did a 'cd ..; mkdir tmp; cd tmp; git-init-db' as before, but there
is still no ".git/info" entry created.

Be well,
Zack

> 
> As I said in a previous message, the first paragraph in INSTALL
> file explains this.
> 

-- 
Zack Brown

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Junio C Hamano @ 2006-04-20 20:17 UTC (permalink / raw)
  To: Zack Brown; +Cc: git
In-Reply-To: <20060420200849.GA3653@tumblerings.org>

Zack Brown <zbrown@tumblerings.org> writes:

> Not true. I went into the git source directory, and ran "make". Nothing more.

Ah, I misunderstood.  You are trying to run it _without_
installing it.

Well, then probably you do not have templates installed
anywhere, especially not where git-init-db expects them to be
found.

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Petr Baudis @ 2006-04-20 20:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Zack Brown, git
In-Reply-To: <7vslo8xaql.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Thu, Apr 20, 2006 at 10:17:38PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Zack Brown <zbrown@tumblerings.org> writes:
> 
> > Not true. I went into the git source directory, and ran "make". Nothing more.
> 
> Ah, I misunderstood.  You are trying to run it _without_
> installing it.
> 
> Well, then probably you do not have templates installed
> anywhere, especially not where git-init-db expects them to be
> found.

Duh, but shouldn't git-init-db create .git/info at any rate, even when
no templates are installed?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Junio C Hamano @ 2006-04-20 20:23 UTC (permalink / raw)
  To: Zack Brown; +Cc: git
In-Reply-To: <7vslo8xaql.fsf@assigned-by-dhcp.cox.net>

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

> Zack Brown <zbrown@tumblerings.org> writes:
>
>> Not true. I went into the git source directory, and ran "make". Nothing more.
>
> Ah, I misunderstood.  You are trying to run it _without_
> installing it.
>
> Well, then probably you do not have templates installed
> anywhere, especially not where git-init-db expects them to be
> found.

(sorry for the short message sent unfinished by mistake).

Running things without installing is somewhat tricky, but test
framework needs to do that, so there are some things you would
need to do.

 - "git init-db" takes --template argument; in the source area
   before installing, they are built in templates/blt/.

 - "git" and programs that need to invoke other git programs
   (e.g. git-send-pack) expects things to be found in gitexecdir
   you set when you build.  If you are not installing, you need
   to override that with GIT_EXEC_PATH environment variable.

There might be other things, but you should be able to find them
from what t/Makefile and t/test-lib.sh do.

Having said that, honestly I would not recommend "runnning
without installing" unless you know what you are doing ;-).

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Junio C Hamano @ 2006-04-20 20:26 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060420201915.GF27689@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> Duh, but shouldn't git-init-db create .git/info at any rate, even when
> no templates are installed?

I do not think so.  We tend to lazily create necessary
directories under .git/ these days, and absolute minimum git
should not need an empty .git/info directory.

If there is something that creates files in .git/info without
making sure that leading path exists, we should fix it (maybe
update-server-info forgets it?  I haven't checked).

^ permalink raw reply

* Re: cg-clone produces "___" file and no working tree
From: Petr Baudis @ 2006-04-20 20:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk69kxabp.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Thu, Apr 20, 2006 at 10:26:34PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Duh, but shouldn't git-init-db create .git/info at any rate, even when
> > no templates are installed?
> 
> I do not think so.  We tend to lazily create necessary
> directories under .git/ these days, and absolute minimum git
> should not need an empty .git/info directory.
> 
> If there is something that creates files in .git/info without
> making sure that leading path exists, we should fix it (maybe
> update-server-info forgets it?  I haven't checked).

Aww. cg-clone assumed that .git/info is canonical part of git repository
now and git-init-db will always creat eit, but now it seems to be the
case only for .git/objects/info.

I've fixed cg-clone. Thanks for the info.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

^ permalink raw reply

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Nicolas Pitre @ 2006-04-20 21:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8xq0yteb.fsf@assigned-by-dhcp.cox.net>

On Thu, 20 Apr 2006, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > On Thu, 20 Apr 2006, Shawn Pearce wrote:
> >
> >> The more that I think about it the more it seems possible that the
> >> pathname hashing is what may be causing the problem.  Not only did
> >> bisect point to 1d6b38cc76c348e2477506ca9759fc241e3d0d46 but the
> >> directory which contains the bulk of the space has many files with
> >> the same name located in different directories:
> > [...]
> >
> > But the bad commit according to your bisection talks about "thin" packs 
> > which are not involved in your case.  So something looks fishy with that 
> > commit which should not have touched path hashing in the non-thin pack 
> > case...  I think...
> 
> I think this explains it.  The new code hashes full-path, but
> places bins for the paths with the same basename next to each
> other, so before Makefile and doc/Makefile and t/Makefile were
> all in the same bin, but now they are in three different bins
> next to each other.

That is fine.  In fact I did try with a tweaked name_hash() that 
completely ignored all directory components and the resulting pack was 
even bigger, much bigger, when repacking Shawn's repo.

> I originally thought, with one single notable exception of
> Makefile, having the identically named file in many different
> directories is not common nor sane,

I'd tend to disagree with that but...

> and the new code favors to
> delta with the exact same path for deeper history over wasting
> delta window for making delta with objects with the same name in
> different places in more recent history.  I think I benched this
> with kernel repository (git.git was too small for that).

This is obviously fine.  And if a file in a given directory has few 
revisions then the delta window will consider objects for a file with 
the same name in other directories as well, which is also sensible.  So 
if files of the same name are located in different directories they 
should delta well against each other if they're similar enough.  This 
should cover Shawn's repo layout.

> But I suspect we have a built-in "we sort bigger to smaller, and
> we cut off when we switch bins" somewhere in find_delta() loop,
> which I do not recall touching when I did that change, so that
> may be interfering and preventing 0-11-AdjLite.deg from all over
> the place to delta against each other.

I just cannot find something that would do that in the code.  When 
--no-reuse-delta is specified, the only things that will break the loop
in find_delta() is when try_delta() returns -1, and that happens only 
when changing object type or when the size difference is too big, but 
nothing looks at the name hash.

It is also hard to corelate it with commit 1d6b38cc which is the one 
that introduced the regression.


Nicolas

^ permalink raw reply

* [PATCH] fix pack-object buffer size
From: Nicolas Pitre @ 2006-04-20 21:25 UTC (permalink / raw)
  To: git

The input line has 40 _chars_ of sha1 and no 20 _bytes_. It should also 
account for the space before the pathname, and the terminating \n and \0.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

I doubt anyone has ever used a repository with paths long enough to hit 
the limit, but better make it right nevertheless.

diff --git a/pack-objects.c b/pack-objects.c
index 09f4f2c..3c2767b 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -1231,7 +1231,7 @@ static void setup_progress_signal(void)
 int main(int argc, char **argv)
 {
 	SHA_CTX ctx;
-	char line[PATH_MAX + 20];
+	char line[40 + 1 + PATH_MAX + 2];
 	int window = 10, depth = 10, pack_to_stdout = 0;
 	struct object_entry **list;
 	int num_preferred_base = 0;

^ permalink raw reply related

* Re: 1.3.0 creating bigger packs than 1.2.3
From: Junio C Hamano @ 2006-04-20 21:31 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git, Nicolas Pitre, Linus Torvalds
In-Reply-To: <20060420173131.GF31738@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> I just spent some time bisecting this issue and it looks like the
> following change by Junio may be the culprit:
>
>   commit 1d6b38cc76c348e2477506ca9759fc241e3d0d46
>   Author: Junio C Hamano <junkio@cox.net>
>   Date:   Wed Feb 22 22:10:24 2006 -0800
>   
>       pack-objects: use full pathname to help hashing with "thin" pack.
>       
>       This uses the same hashing algorithm to the "preferred base
>       tree" objects and the incoming pathnames, to group the same
>       files from different revs together, while spreading files with
>       the same basename in different directories.
>       
>       Signed-off-by: Junio C Hamano <junkio@cox.net>
>   

Unfortunately, that is not the same hash we use in v1.3.0, so we
need to look elsewhere for interactions.

v1.2.3 hash was base-name only.  doc/Makefile and t/Makefile
were thrown in the same bin and sorted by size.  When the
history you are packing is deep, and doc/Makefile and t/Makefile
are not related at all, this made effective size of delta window
1/N where N is the number of such duplicates.

The one you found above uses a hash that is fully full-path.
The two are in completely different bins, and bins are totally
random.  This was not a good strategy.

v1.3.0 hash is base-name hash concatenated with leading-path
has.  t/Makefile and doc/Makefile go in separate bins, but the
bins are close to each other; this avoids the problem in v1.2.3
when you have deep history, but at the same time if you do not
have many many versions of t/Makefile to overflow the delta
window, it gives t/Makefile a chance to delta with doc/Makefile.

The earlier observation by Linus on reverting eeef7135 is
consistent with it; that commit was the one that introduced
v1.3.0 hash.

You could try this patch to resurrect the hash used in v1.2.3,
and you may get better packing for your particular repository;
but I am not sure if it gives better results in the general
case.  I am running the test myself now while waiting for my
day-job database to load X-<.

NOTE NOTE NOTE.  The hash in v1.2.3 was done with the basename
(relying on rev-list --objects to only show the basename) and
hashed from front to back.  The current one uses the same hash
scrambling function, but it hashes from back to front, and it
knows rev-list --objects gives it a full path.

What this patch does is to stop the hashing after we are done
with the basename part.  So it still gives different hash value
to the same path from v1.2.3 version, but the distribution
should be equivalent.

NOTE 2.  Feeding output from the current "rev-list --objects" to
v1.2.3 pack-object is the same as "hash full path and spread
things out" intermediate version, which is the worst performer.

-- >8 --
git diff
diff --git a/pack-objects.c b/pack-objects.c
index 09f4f2c..e58e169 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -492,6 +492,8 @@ static unsigned name_hash(struct name_pa
 		name_hash = hash;
 		hash = 0;
 	}
+	return name_hash;
+
 	for (p = path; p; p = p->up) {
 		hash = hash * 11 + '/';
 		n = p->elem + p->len;

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox