* Re: [RFC] Cache negative delta pairs
From: Nicolas Pitre @ 2006-06-29 21:24 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606291352110.12404@g5.osdl.org>
On Thu, 29 Jun 2006, Linus Torvalds wrote:
> Instead of having a separate cache, wouldn't it be much better to just
> take the hint from the previous pack-file?
DOH! ;-)
Nicolas
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Linus Torvalds @ 2006-06-29 21:04 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606291616480.1213@localhost.localdomain>
On Thu, 29 Jun 2006, Nicolas Pitre wrote:
>
> The negative delta cache concept is certainly attractive even for normal
> repositories, especially for public servers, since when used in
> conjonction with delta reuse it makes the creation of a pack basically
> free. So I think this idea really has merits, as long as the cache
> remains small.
I don't really see much of a point of this all.
Instead of having a separate cache, wouldn't it be much better to just
take the hint from the previous pack-file?
In the repacking window, if both objects we are looking at already came
from the same (old) pack-file, don't bother delta'ing them against each
other.
That means that we'll still always check for better deltas for (and
against!) _unpacked_ objects, but assuming incremental repacks, you'll
avoid the delta creation 99% of the time.
Ie somethng really simple like the appended.
Linus
---
diff --git a/pack-objects.c b/pack-objects.c
index bed2497..cea63e7 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -988,6 +988,13 @@ static int try_delta(struct unpacked *tr
return -1;
/*
+ * We do not bother to try a delta that we discarded
+ * on an earlier try
+ */
+ if (trg_entry->in_pack && trg_entry->in_pack == src_entry->in_pack)
+ return -1;
+
+ /*
* If the current object is at pack edge, take the depth the
* objects that depend on the current object into account --
* otherwise they would become too deep.
^ permalink raw reply related
* Re: [PATCH] move get_merge_bases() to core lib; use it in merge-recursive
From: Junio C Hamano @ 2006-06-29 20:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606292050380.29667@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> So, a few tests later, I am pretty sure that my patches do not break
> git-merge-base. I'll prepare another patch series which builds-in
> merge-base.
I'll take your two patches to refactor merge-base and to move
the bulk of the code to commit.c for now. Updated "next" coming
shortly.
^ permalink raw reply
* Re: rebasing trouble
From: J. Bruce Fields @ 2006-06-29 20:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20060629202234.GE14287@fieldses.org>
On Thu, Jun 29, 2006 at 04:22:34PM -0400, bfields wrote:
> I realized after I sent that that "origin" in that public repository actually
> isn't the same as "origin" in the local repo that I'm working on. So I just
> ran a GIT_DIR=. git-fetch there, and now both branches agree.
By the way, I should also have said:
bfields@puzzle:git$ git --version
git version 1.4.1.rc2
--b.
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Nicolas Pitre @ 2006-06-29 20:24 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20060629195201.GA10786@coredump.intra.peff.net>
On Thu, 29 Jun 2006, Jeff King wrote:
> On Thu, Jun 29, 2006 at 03:04:15PM -0400, Nicolas Pitre wrote:
>
> > Right. Your use pattern is a special case that doesn't work well with
> > the whole window hash approach. I'd expect it to work beautifully with
> > the kernel repository though.
>
> I don't necessarily care about the kernel repository. It packs fine as
> it is, and you only waste 30 seconds on a repack checking deltas that
> could be avoided. I do care on my special repository where packing is
> virtually unusable and I can achieve a 45x speedup. Maybe my original
> caching is not worth it for the kernel and should be configurable,
> but obviously this window caching cannot REPLACE mine since it fails
> utterly for the one thing I wanted it for.
And I agreed with you already.
> That being said, I'm not sure that window caching is all that great for
> "normal" repos.
>
> Same test as before, but instead of simulating the commits, I merely
> looked at the window hashes produced by
> git-rev-list --objects master~$x
>
> For the git repo:
> x=0 tries 6698 windows
> x=0 and x=50 contain 5197 identical windows
> x=0 and x=100 contain 2484 identical windows
> x=0 and x=500 contain 455 identical windows
>
> For linux-2.6 repo:
> x=0 tries 57208 windows
> x=0 and x=50 contain 53677 identical windows
> x=0 and x=100 contain 52886 identical windows
> x=0 and x=500 contain 41196 identical windows
>
> Obviously the kernel repo is doing better, but x=500 is only 4 days ago.
> Trying with --before=2.weeks.ago yields only 31505 matches.
What does this prove? I fail to see the relation between those results
and a possible git-pack-objects improvement.
The negative delta cache concept is certainly attractive even for normal
repositories, especially for public servers, since when used in
conjonction with delta reuse it makes the creation of a pack basically
free. So I think this idea really has merits, as long as the cache
remains small.
Nicolas
^ permalink raw reply
* Re: rebasing trouble
From: J. Bruce Fields @ 2006-06-29 20:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd5crranq.fsf@assigned-by-dhcp.cox.net>
On Thu, Jun 29, 2006 at 01:04:25PM -0700, Junio C Hamano wrote:
> "J. Bruce Fields" <bfields@fieldses.org> writes:
>
> > I must be missing something obvious:
> >
> > bfields@puzzle:linux-2.6$ git checkout -b TMP nfs-client-stable^^^
> > bfields@puzzle:linux-2.6$ git-describe
> > v2.6.17-rc6-g28df955
> > bfields@puzzle:linux-2.6$ git-rebase --onto v2.6.17 origin
> > Nothing to do.
> > bfields@puzzle:linux-2.6$ git-describe
> > v2.6.17
> >
> > So the git-rebase just reset TMP to v2.6.17. But I know that nfs-client-stable
> > isn't a subset of origin, so this doesn't make sense to me.
> >
> > The tree in question is actually at git://linux-nfs.org/~bfields/linux.git, if
> > it matters.
>
> It matters of course. Where is the "origin"?
I realized after I sent that that "origin" in that public repository actually
isn't the same as "origin" in the local repo that I'm working on. So I just
ran a GIT_DIR=. git-fetch there, and now both branches agree.
--b.
^ permalink raw reply
* Re: [PATCH 2/2] Rename man1 and man7 variables to man1dir and man7dir
From: Junio C Hamano @ 2006-06-29 20:17 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200606292213.00726.jnareb@gmail.com>
Doesn't this break "make dist-doc" by _REMOVING_ make variables
man1 and man7, which I mentioned earlier?
^ permalink raw reply
* Re: [PATCH] Allow INSTALL, bindir, mandir to be set in main Makefile
From: Jakub Narebski @ 2006-06-29 20:16 UTC (permalink / raw)
To: git
In-Reply-To: <7vy7vfrfc6.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> Part of autoconf series, but independent.
>
> I'd like to take something like this, independently from
> "optionally managing config.mak with autoconf" series.
>
>> Should probably be split into two patches:
>> * first with export + '?='
>> * second renaming man1 and man7 to man1dir and man7dir
>
> And I think it is probably a good idea to somehow keep people's
> configurations that have been overriding man1 and man7 if
> possible. Otherwise things would regress for them.
Done. Resent as two patch "series", starting with
Message-Id: <200606292211.28352.jnareb@gmail.com>
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* [PATCH 2/2] Rename man1 and man7 variables to man1dir and man7dir
From: Jakub Narebski @ 2006-06-29 20:13 UTC (permalink / raw)
To: git
In-Reply-To: <200606292211.28352.jnareb@gmail.com>
This patch renames man1 and man7 variables to man1dir and man7dir,
according to "Makefile Conventions: Variables for Installation
Directories" in make.info of GNU Make.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Depends on the earlier patch in "series" only via context
Documentation/Makefile | 10 +++++-----
contrib/git-svn/Makefile | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index ca6b77d..cc83610 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -27,8 +27,8 @@ DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT
prefix?=$(HOME)
bindir?=$(prefix)/bin
mandir?=$(prefix)/man
-man1=$(mandir)/man1
-man7=$(mandir)/man7
+man1dir=$(mandir)/man1
+man7dir=$(mandir)/man7
# DESTDIR=
INSTALL?=install
@@ -52,9 +52,9 @@ man1: $(DOC_MAN1)
man7: $(DOC_MAN7)
install: man
- $(INSTALL) -d -m755 $(DESTDIR)$(man1) $(DESTDIR)$(man7)
- $(INSTALL) $(DOC_MAN1) $(DESTDIR)$(man1)
- $(INSTALL) $(DOC_MAN7) $(DESTDIR)$(man7)
+ $(INSTALL) -d -m755 $(DESTDIR)$(man1dir) $(DESTDIR)$(man7dir)
+ $(INSTALL) $(DOC_MAN1) $(DESTDIR)$(man1dir)
+ $(INSTALL) $(DOC_MAN7) $(DESTDIR)$(man7dir)
#
diff --git a/contrib/git-svn/Makefile b/contrib/git-svn/Makefile
index 1a6585e..8cac688 100644
--- a/contrib/git-svn/Makefile
+++ b/contrib/git-svn/Makefile
@@ -3,7 +3,7 @@ all: git-svn
prefix?=$(HOME)
bindir?=$(prefix)/bin
mandir?=$(prefix)/man
-man1=$(mandir)/man1
+man1dir=$(mandir)/man1
INSTALL?=install
doc_conf=../../Documentation/asciidoc.conf
-include ../../config.mak
@@ -17,7 +17,7 @@ install: all
$(INSTALL) git-svn $(DESTDIR)$(bindir)
install-doc: doc
- $(INSTALL) git-svn.1 $(DESTDIR)$(man1)
+ $(INSTALL) git-svn.1 $(DESTDIR)$(man1dir)
doc: git-svn.1
git-svn.1 : git-svn.xml
--
1.4.0
^ permalink raw reply related
* [PATCH 1/2] Allow INSTALL, bindir, mandir to be set in main Makefile
From: Jakub Narebski @ 2006-06-29 20:11 UTC (permalink / raw)
To: git
Makefiles in subdirectories now use existing value of INSTALL, bindir,
mandir if it is set, allowing those to be set in main Makefile or in
included config.mak. Main Makefile exports variables which it sets.
Accidentally it renames bin to bindir in Documentation/Makefile
(should be bindir from start, but is unused, perhaps to be removed).
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Series only because second patch in series depends textually on first,
even though changes are independent.
Documentation/Makefile | 4 ++--
Makefile | 2 ++
contrib/emacs/Makefile | 4 ++--
contrib/git-svn/Makefile | 4 ++--
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2b0efe7..ca6b77d 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -25,8 +25,8 @@ DOC_MAN1=$(patsubst %.txt,%.1,$(MAN1_TXT
DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
prefix?=$(HOME)
-bin=$(prefix)/bin
-mandir=$(prefix)/man
+bindir?=$(prefix)/bin
+mandir?=$(prefix)/man
man1=$(mandir)/man1
man7=$(mandir)/man7
# DESTDIR=
diff --git a/Makefile b/Makefile
index cde619c..b8fe669 100644
--- a/Makefile
+++ b/Makefile
@@ -100,6 +100,8 @@ template_dir = $(prefix)/share/git-core/
GIT_PYTHON_DIR = $(prefix)/share/git-core/python
# DESTDIR=
+export prefix bindir gitexecdir template_dir GIT_PYTHON_DIR
+
CC = gcc
AR = ar
TAR = tar
diff --git a/contrib/emacs/Makefile b/contrib/emacs/Makefile
index d3619db..350846d 100644
--- a/contrib/emacs/Makefile
+++ b/contrib/emacs/Makefile
@@ -3,9 +3,9 @@ ## Build and install stuff
EMACS = emacs
ELC = git.elc vc-git.elc
-INSTALL = install
+INSTALL ?= install
INSTALL_ELC = $(INSTALL) -m 644
-prefix = $(HOME)
+prefix ?= $(HOME)
emacsdir = $(prefix)/share/emacs/site-lisp
all: $(ELC)
diff --git a/contrib/git-svn/Makefile b/contrib/git-svn/Makefile
index 7c20946..1a6585e 100644
--- a/contrib/git-svn/Makefile
+++ b/contrib/git-svn/Makefile
@@ -1,8 +1,8 @@
all: git-svn
prefix?=$(HOME)
-bindir=$(prefix)/bin
-mandir=$(prefix)/man
+bindir?=$(prefix)/bin
+mandir?=$(prefix)/man
man1=$(mandir)/man1
INSTALL?=install
doc_conf=../../Documentation/asciidoc.conf
--
1.4.0
^ permalink raw reply related
* Re: rebasing trouble
From: Junio C Hamano @ 2006-06-29 20:04 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: git
In-Reply-To: <20060629194723.GD14287@fieldses.org>
"J. Bruce Fields" <bfields@fieldses.org> writes:
> I must be missing something obvious:
>
> bfields@puzzle:linux-2.6$ git checkout -b TMP nfs-client-stable^^^
> bfields@puzzle:linux-2.6$ git-describe
> v2.6.17-rc6-g28df955
> bfields@puzzle:linux-2.6$ git-rebase --onto v2.6.17 origin
> Nothing to do.
> bfields@puzzle:linux-2.6$ git-describe
> v2.6.17
>
> So the git-rebase just reset TMP to v2.6.17. But I know that nfs-client-stable
> isn't a subset of origin, so this doesn't make sense to me.
>
> The tree in question is actually at git://linux-nfs.org/~bfields/linux.git, if
> it matters.
It matters of course. Where is the "origin"?
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Jeff King @ 2006-06-29 19:52 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606291458110.1213@localhost.localdomain>
On Thu, Jun 29, 2006 at 03:04:15PM -0400, Nicolas Pitre wrote:
> Right. Your use pattern is a special case that doesn't work well with
> the whole window hash approach. I'd expect it to work beautifully with
> the kernel repository though.
I don't necessarily care about the kernel repository. It packs fine as
it is, and you only waste 30 seconds on a repack checking deltas that
could be avoided. I do care on my special repository where packing is
virtually unusable and I can achieve a 45x speedup. Maybe my original
caching is not worth it for the kernel and should be configurable,
but obviously this window caching cannot REPLACE mine since it fails
utterly for the one thing I wanted it for.
That being said, I'm not sure that window caching is all that great for
"normal" repos.
Same test as before, but instead of simulating the commits, I merely
looked at the window hashes produced by
git-rev-list --objects master~$x
For the git repo:
x=0 tries 6698 windows
x=0 and x=50 contain 5197 identical windows
x=0 and x=100 contain 2484 identical windows
x=0 and x=500 contain 455 identical windows
For linux-2.6 repo:
x=0 tries 57208 windows
x=0 and x=50 contain 53677 identical windows
x=0 and x=100 contain 52886 identical windows
x=0 and x=500 contain 41196 identical windows
Obviously the kernel repo is doing better, but x=500 is only 4 days ago.
Trying with --before=2.weeks.ago yields only 31505 matches.
So the windows do clearly experience a fair bit of churn, but whether or
not this is worth it depends on how long you think is reasonable before
something gets "churned out" .
-Peff
^ permalink raw reply
* rebasing trouble
From: J. Bruce Fields @ 2006-06-29 19:47 UTC (permalink / raw)
To: Git Mailing List
I must be missing something obvious:
bfields@puzzle:linux-2.6$ git checkout -b TMP nfs-client-stable^^^
bfields@puzzle:linux-2.6$ git-describe
v2.6.17-rc6-g28df955
bfields@puzzle:linux-2.6$ git-rebase --onto v2.6.17 origin
Nothing to do.
bfields@puzzle:linux-2.6$ git-describe
v2.6.17
So the git-rebase just reset TMP to v2.6.17. But I know that nfs-client-stable
isn't a subset of origin, so this doesn't make sense to me.
The tree in question is actually at git://linux-nfs.org/~bfields/linux.git, if
it matters.
--b.
^ permalink raw reply
* Re: [PATCH] move get_merge_bases() to core lib; use it in merge-recursive
From: Johannes Schindelin @ 2006-06-29 19:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmzbvrela.fsf@assigned-by-dhcp.cox.net>
Hi Linus, Hi Junio
[this is a response to both of your responses; mail threads cannot yet be
merged a la git ;-)]
On Thu, 29 Jun 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > My point being: it makes no sense to split off get_merge_bases() if nobody
> > uses it except for git-merge-base.
>
> I do not think that is a good reasoning. If something is
> reusable (or you made it reusable) and you are planning to reuse
> it later, splitting it out without changing anything else to
> make sure the split is correct is a seemingly small but a very
> important step.
Okay. Convinced.
I tested my patch again, and like Alex said, a test fails. But I tested on
top of Alex's latest merge-recursive patch, which has that nasty
update-index bug, and that is the reason for the test to fail.
So, a few tests later, I am pretty sure that my patches do not break
git-merge-base. I'll prepare another patch series which builds-in
merge-base.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Nicolas Pitre @ 2006-06-29 19:06 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20060629185759.GB6704@coredump.intra.peff.net>
On Thu, 29 Jun 2006, Jeff King wrote:
> On Thu, Jun 29, 2006 at 02:48:23PM -0400, Nicolas Pitre wrote:
>
> > Dare to test it? I provided you with most of the code difference
> > already.
>
> See my other mail. Unless I did something horribly wrong, caching full
> windows is largely useless.
... on your special photo repository I agree.
I'm still unconvinced for large repos though.
Nicolas
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Nicolas Pitre @ 2006-06-29 19:04 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20060629185335.GA6704@coredump.intra.peff.net>
On Thu, 29 Jun 2006, Jeff King wrote:
> On Thu, Jun 29, 2006 at 02:24:57PM -0400, Nicolas Pitre wrote:
>
> > > I assumed the window would change over time (though our total is still
> > > likely to hang around N*10 rather than N^2).
> > It doesn't change unless you force a different window size.
>
> Sorry, I meant "the items in the window for a given object would change
> over time."
>
> > > This will fail to hit the cache anytime the window changes. How often
> > > does the window change? In my test case, I would think anytime I added a
> > > bunch of new photos, it would be likely that one of them would make it
> > > into the window, thus invalidating the cache entry and forcing me to try
> > > against every object in the window (even though I've already tried
> > > 9/10).
> > Sure. But on the lot how often will that happen?
>
> Reasonably often, according to my test. I did this to simulate usage
> over time:
> - create an empty repo
> - from my test repo of 515 images, grab 20 at a time and add/commit
> them
> - after each commit, record the SHA1 of (object, window[0..n]) for
> each object to be delta'd
> If doing the cache on the sha1 of the whole window is a good idea, then
> we should see many of the same hashes from commit to commit. If we
> don't, that means the newly added files are being placed in the old
> windows, thus disrupting their hashes.
>
> The results were that there was typically only 1 reusable window each
> time I added 20 files. At that point, caching is largely pointless.
Right. Your use pattern is a special case that doesn't work well with
the whole window hash approach. I'd expect it to work beautifully with
the kernel repository though.
> > And even then, since my suggested method implies only one cache lookup
> > in a much smaller cache instead of 10 lookups in a larger cache for each
> > objects it might end up faster overall even if sometimes some windows
> > don't match and deltas are recomputed needlessly.
>
> I didn't benchmark, but I doubt it will have significant impact.
> Especially on my photo test repo, the lookups are dominated by the
> create_delta time by several orders of magnitude.
Again I think it is a repo like the linux kernel that would benefit
more.
> > Of course a greater depth might allow for a hit where there isn't any
> > otherwise. But changing the delta depth is not something someone does
> > that often, and when the depth is changed then you better use -f with
> > git-repack as well which like I said should also ignore the cache.
>
> That sounds reasonable to me for depth. What about other reasons for
> try_delta to fail? Preferred base?
Hmmm. That might need to be dealth with (easily but still).
Nicolas
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Jeff King @ 2006-06-29 18:58 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606291444370.1213@localhost.localdomain>
On Thu, Jun 29, 2006 at 02:48:23PM -0400, Nicolas Pitre wrote:
> Dare to test it? I provided you with most of the code difference
> already.
See my other mail. Unless I did something horribly wrong, caching full
windows is largely useless.
> And what do you mean by "semantically correct"?
I mean that right now the cache means "calling create_delta on content
that has sha1_a against content that has sha1_b will not produce a
useful delta." That makes sense since the content is never going to
change for a given sha1. However, covering the whole window takes into
account depth and preferred base. We just need to be sure that it is
correct to be including those in our cache calculation (I don't know,
and I'll defer to your judgement on that, since you clearly know more
about the delta logic than I do).
-Peff
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Jeff King @ 2006-06-29 18:53 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0606291410420.1213@localhost.localdomain>
On Thu, Jun 29, 2006 at 02:24:57PM -0400, Nicolas Pitre wrote:
> > I assumed the window would change over time (though our total is still
> > likely to hang around N*10 rather than N^2).
> It doesn't change unless you force a different window size.
Sorry, I meant "the items in the window for a given object would change
over time."
> > This will fail to hit the cache anytime the window changes. How often
> > does the window change? In my test case, I would think anytime I added a
> > bunch of new photos, it would be likely that one of them would make it
> > into the window, thus invalidating the cache entry and forcing me to try
> > against every object in the window (even though I've already tried
> > 9/10).
> Sure. But on the lot how often will that happen?
Reasonably often, according to my test. I did this to simulate usage
over time:
- create an empty repo
- from my test repo of 515 images, grab 20 at a time and add/commit
them
- after each commit, record the SHA1 of (object, window[0..n]) for
each object to be delta'd
If doing the cache on the sha1 of the whole window is a good idea, then
we should see many of the same hashes from commit to commit. If we
don't, that means the newly added files are being placed in the old
windows, thus disrupting their hashes.
The results were that there was typically only 1 reusable window each
time I added 20 files. At that point, caching is largely pointless.
> And even then, since my suggested method implies only one cache lookup
> in a much smaller cache instead of 10 lookups in a larger cache for each
> objects it might end up faster overall even if sometimes some windows
> don't match and deltas are recomputed needlessly.
I didn't benchmark, but I doubt it will have significant impact.
Especially on my photo test repo, the lookups are dominated by the
create_delta time by several orders of magnitude.
> Of course a greater depth might allow for a hit where there isn't any
> otherwise. But changing the delta depth is not something someone does
> that often, and when the depth is changed then you better use -f with
> git-repack as well which like I said should also ignore the cache.
That sounds reasonable to me for depth. What about other reasons for
try_delta to fail? Preferred base?
> > > So given my GIT repository such a cache would be 7610 * 40 = 304400
> > > bytes if we stick to the full 40 bytes of sha1 to hash bad combinations.
> > Keep in mind that it will grow every time the window changes.
> What do you mean by window change?
I meant that when the window we use for a given object changes, it will
insert a new cache entry. But if we deal with invalidating unused cache
entries as you suggested before, it won't matter.
-Peff
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Nicolas Pitre @ 2006-06-29 18:48 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20060629180719.GB4392@coredump.intra.peff.net>
On Thu, 29 Jun 2006, Jeff King wrote:
> On Thu, Jun 29, 2006 at 12:39:31PM -0400, Nicolas Pitre wrote:
>
> > You do that lookup for every delta match attempt. Instead it could be
> > done once for the whole window attempt, potentially reducing the cache
> > size by a factor of 20, and it might be faster too.
>
> I'm not convinced this will provide good cache hit characteristics, and
> I'm not convinced it's semantically correct (see my other mail).
Dare to test it? I provided you with most of the code difference
already.
And what do you mean by "semantically correct"?
Nicolas
^ permalink raw reply
* Re: CFT: merge-recursive in C (updated)
From: Johannes Schindelin @ 2006-06-29 18:40 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, git, Fredrik Kuivinen, Linus Torvalds
In-Reply-To: <81b0412b0606290143g5a3a0f5atbda3f4250411e92e@mail.gmail.com>
Hi,
On Thu, 29 Jun 2006, Alex Riesen wrote:
> [cc list restored, I'm lost in the maze of git update-index, all cache
> changing functions looking almost the same]
>
> On 6/29/06, Junio C Hamano <junkio@cox.net> wrote:
> > > this broke t6022-merge-rename.sh (the second test). It produces an
> > > index with this:
> > >
> > > .../t/trash$ git-diff-index white
> > > :100644 100644 2d603156dc5bdf6295c789cac08e3c9942a0b82a
> > 0000000000000000000000000000000000000000 M B
> > > :100644 100644 ba41fb96393979b22691106b06bf5231eab57b85
> > 0000000000000000000000000000000000000000 M N
> > >
> > > whereas git-merge-recursive (and the previous version, without pipe):
> > >
> > > .../t/trash$ git-diff-index white
> > > :100644 100644 2d603156dc5bdf6295c789cac08e3c9942a0b82a
> > 0000000000000000000000000000000000000000 M B
> > >
> > > I can see that "git update-index --add" is somehow different from a
> > > pipe to "git update-index --index-info", but not very clear. Does this
> > > "zero-sha1" mean that the file "N" is not in the index?
> >
> > When diff-index and diff-files compare a tree entry or an index
> > entry with a file in the working tree, they do not compute the
> > blob hash value for the file in the working tree. 0{40} is used
> > on the RHS in such a case. When the working tree file matches
> > the corresponding index entry, then we know RHS matches what is
> > in the index, so both sides have the blob hash value.
>
> Ok. Am I correct in the assumption, that if the file in working tree has
> the same SHA1 as LHS, than the next "git-update-index --refresh" will
> remove the entry from git-diff-index output?
> This is what actually happens, if I do "git-update-index --refresh", so I
> suspect that I have an SHA1 update gone missing somewhere.
I think the problem is more like this (in ce_match_stat_basic()):
if (ce->ce_mtime.sec != htonl(st->st_mtime))
changed |= MTIME_CHANGED;
if (ce->ce_ctime.sec != htonl(st->st_ctime))
changed |= CTIME_CHANGED;
If you update with --index-info, the mtime and ctime is not updated
from the file in the working directory.
All the more a reason to go forward with direct calls to read_cache() and
write_cache(). At the moment, my plan is
- trivially split the read_cache() code into read_cache()/read_cache_from(),
- introduce flush_cache(),
- trivially rewrite add_file_to_cache(), and add_cache_info() from
builtin-update-index.c, move that to read-tree.c, too, and
- throw out the pipe to git-update-index from merge-recursive.c
altogether.
This should be less intrusive than it sounds: with the introduction of
read_cache_from() it should be trivial to handle the problem of different
index files.
We have to put a lock_file on the index file at the start, and write the
index file at the end.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] move get_merge_bases() to core lib; use it in merge-recursive
From: Junio C Hamano @ 2006-06-29 18:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606291814200.29667@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> My point being: it makes no sense to split off get_merge_bases() if nobody
> uses it except for git-merge-base.
I do not think that is a good reasoning. If something is
reusable (or you made it reusable) and you are planning to reuse
it later, splitting it out without changing anything else to
make sure the split is correct is a seemingly small but a very
important step.
^ permalink raw reply
* Re: [PATCH] move get_merge_bases() to core lib; use it in merge-recursive
From: Linus Torvalds @ 2006-06-29 18:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alex Riesen, git, Junio C Hamano, Fredrik Kuivinen
In-Reply-To: <Pine.LNX.4.63.0606291814200.29667@wbgn013.biozentrum.uni-wuerzburg.de>
On Thu, 29 Jun 2006, Johannes Schindelin wrote:
>
> My point being: it makes no sense to split off get_merge_bases() if nobody
> uses it except for git-merge-base.
I disagree. Your get_merge_bases() function is another small step in
cleanup and libification, so I think it's perfectly valid regardless of
whether somebody uses it yet.
A lot of the libification effort ends up being preparatory, because often
you need to get past a hump of libifying a _lot_ in order to actually use
something in a library.
So at first, the only user is usually the old separate program. So you
libify get_merge_bases, and initially the only user is git-merge-base,
which could now be made a built-in, since the bulk of its code is going to
be linked in regardless..
Linus
^ permalink raw reply
* Re: [RFC] Cache negative delta pairs
From: Nicolas Pitre @ 2006-06-29 18:24 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20060629180011.GA4392@coredump.intra.peff.net>
On Thu, 29 Jun 2006, Jeff King wrote:
> > This is way suboptimal. First there is no reason for the cache to ever
> > grow to N^2. At worst it should be N*10 where 10 is the current window
> > size.
>
> I assumed the window would change over time (though our total is still
> likely to hang around N*10 rather than N^2).
It doesn't change unless you force a different window size.
> > none of the objects found in a given window. Therefore we only have to
> > compute a hash for the object names found in that window and store that
> > in the cache. So the cache entries would then be a pair of sha1: first
> > the sha1 of the victim object, and the sha1 of all sha1 names for the
> > objects against which the victim object was found not to delta well
> > against.
>
> This will fail to hit the cache anytime the window changes. How often
> does the window change? In my test case, I would think anytime I added a
> bunch of new photos, it would be likely that one of them would make it
> into the window, thus invalidating the cache entry and forcing me to try
> against every object in the window (even though I've already tried
> 9/10).
Sure. But on the lot how often will that happen?
This trades a bit of CPU for much smaller cache which might be worth it.
And even then, since my suggested method implies only one cache lookup
in a much smaller cache instead of 10 lookups in a larger cache for each
objects it might end up faster overall even if sometimes some windows
don't match and deltas are recomputed needlessly.
> Also, is it true to say "if this object did not delta against this
> window, it will never delta?" What about interactions with the depth
> parameter?
Of course a greater depth might allow for a hit where there isn't any
otherwise. But changing the delta depth is not something someone does
that often, and when the depth is changed then you better use -f with
git-repack as well which like I said should also ignore the cache.
> > So given my GIT repository such a cache would be 7610 * 40 = 304400
> > bytes if we stick to the full 40 bytes of sha1 to hash bad combinations.
>
> Keep in mind that it will grow every time the window changes.
What do you mean by window change?
Nicolas
^ permalink raw reply
* Re: [PATCH] autoconf: Use autoconf to write installation directories to config.mak
From: Junio C Hamano @ 2006-06-29 18:23 UTC (permalink / raw)
To: Matthias Lederhofer; +Cc: git, Jakub Narebski
In-Reply-To: <E1FvvuX-0002Lr-Nt@moooo.ath.cx>
Matthias Lederhofer <matled@gmx.net> writes:
>> This is beginning of patch series introducing installation configuration
>> using autoconf (and no other autotools) to git. The idea is to generate
>> config.mak using ./configure (generated from configure.ac) from
>> config.mak.in, so one can use autoconf as an _alternative_ to ordinary
>> Makefile, and creating one's own config.mak.
>
> Are you sure this should be named config.mak? From INSTALL:
>> You can place local settings in config.mak and the Makefile
>> will include them. Note that config.mak is not distributed;
>> the name is reserved for local settings.
>
> So with another filename either include it
> - before config.mak: the user may override ./configure options with
> config.mak
> - after config.mak: ./configure overrides config.mak
> At least do not overwrite config.mak if it exists.
Think of it as an incredibly smart editor that you can use to
edit your config.mak. As far as I understand it, this is an
"opt-in" way for you to manage config.mak without editing it
yourself, so I do not have much objections to the series in
principle.
I think it is fair to say that autoconf is not so bad compared
to its worse friends, but I am biased -- I used to deal with
autoconf quite a lot in old days (before there were automake nor
libtool), and my own changes to autoconf might still be
surviving in a few .m4 files but I doubt it it was so long ago.
Having said that, I am skeptical how well we can keep this
contained only to config.mak and keep it "opt-in", so I am not
enthused to have this series at the toplevel of the tree.
It would have been a bit easier to swallow if this whole
machinery to build config.mk were somewhere under contrib/ (say
in contrib/autoconf), with an instruction to make an "opt-in"
symlink "ln -s contrib/autoconf/config.mk config.mk" for people
who want to use it in the toplevel INSTALL file, perhaps.
The one that touches the Makefile to propagate some variables
down to submakes is probably a desirable change but that is
pretty much independent from the series.
^ permalink raw reply
* Re: [PATCH] Allow INSTALL, bindir, mandir to be set in main Makefile
From: Junio C Hamano @ 2006-06-29 18:23 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200606291835.53788.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Part of autoconf series, but independent.
I'd like to take something like this, independently from
"optionally managing config.mak with autoconf" series.
> Should probably be split into two patches:
> * first with export + '?='
> * second renaming man1 and man7 to man1dir and man7dir
And I think it is probably a good idea to somehow keep people's
configurations that have been overriding man1 and man7 if
possible. Otherwise things would regress for them.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox