[RFC/PATCH 0/3] Teach builtin-clone to pack refs

[RFC/PATCH 0/3] Teach builtin-clone to pack refs

[Johan Herland]

The following series builds on top of Barkalow's existing builtin-clone work, available in the "builtin-clone" branch at:
	git://iabervon.org/~barkalow/git.git

This patch series teaches builtin-clone to create packed refs. Creating packed refs directly in clone (instead of creating loose refs and have the next "git gc" (re)pack them), makes cloning considerably faster on repos with many refs. In a test repo with 11000 refs (1000 branches, 10000 tags) I get the following numbers (Core 2 Quad, 4GB RAM running Gentoo Linux):

- Current "next": ~24.8 seconds
- Barkalow's "builtin-clone" branch: 1.47 seconds
- "builtin-clone" plus this series: 0.31 seconds

Although most of the speedup from current "next" is achieved by the builtin-clone work, there is still a considerable additional improvement from writing all refs to a single file instead of writing one file per ref. I expect the performance improvement to be much bigger on platforms with slower filesystem (aka. Windows).

A side-effect of this series is that the cloned refs will not get reflog entries. I don't know how important these "clone: from $URL" entries are to people; I, for one, wouldn't miss them at all.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

[RFC/PATCH 1/3] Move pack_refs() and friends into libgit

[Johan Herland]

This moves pack_refs() and underlying functionality into the library,
to make pack-refs functionality easily available to all git programs.

Most of builtin-pack-refs.c has been moved verbatim into a new file
pack-refs.c that is compiled into libgit.a. A corresponding header
file, pack-refs.h, has also been added, declaring pack_refs() and
the #defines associated with the flags parameter to pack_refs().

This patch introduces no other changes in functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Makefile            |    5 +-
 builtin-pack-refs.c |  121 +--------------------------------------------------
 pack-refs.c         |  117 +++++++++++++++++++++++++++++++++++++++++++++++++
 pack-refs.h         |   18 ++++++++
 4 files changed, 139 insertions(+), 122 deletions(-)
 create mode 100644 pack-refs.c
 create mode 100644 pack-refs.h

diff --git a/Makefile b/Makefile
index 623c639..0804904 100644
--- a/Makefile
+++ b/Makefile
@@ -307,7 +307,8 @@ LIB_H = \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
 	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
-	mailmap.h remote.h parse-options.h transport.h diffcore.h hash.h ll-merge.h fsck.h pack-revindex.h
+	mailmap.h remote.h parse-options.h transport.h diffcore.h hash.h \
+	ll-merge.h fsck.h pack-revindex.h pack-refs.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -331,7 +332,7 @@ LIB_OBJS = \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
 	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
 	transport.o bundle.o walker.o parse-options.o ws.o archive.o branch.o \
-	ll-merge.o alias.o fsck.o pack-revindex.o
+	ll-merge.o alias.o fsck.o pack-revindex.o pack-refs.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 1aaa76d..ff90aef 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -1,125 +1,6 @@
-#include "builtin.h"
 #include "cache.h"
-#include "refs.h"
-#include "object.h"
-#include "tag.h"
 #include "parse-options.h"
-
-struct ref_to_prune {
-	struct ref_to_prune *next;
-	unsigned char sha1[20];
-	char name[FLEX_ARRAY];
-};
-
-#define PACK_REFS_PRUNE	0x0001
-#define PACK_REFS_ALL	0x0002
-
-struct pack_refs_cb_data {
-	unsigned int flags;
-	struct ref_to_prune *ref_to_prune;
-	FILE *refs_file;
-};
-
-static int do_not_prune(int flags)
-{
-	/* If it is already packed or if it is a symref,
-	 * do not prune it.
-	 */
-	return (flags & (REF_ISSYMREF|REF_ISPACKED));
-}
-
-static int handle_one_ref(const char *path, const unsigned char *sha1,
-			  int flags, void *cb_data)
-{
-	struct pack_refs_cb_data *cb = cb_data;
-	int is_tag_ref;
-
-	/* Do not pack the symbolic refs */
-	if ((flags & REF_ISSYMREF))
-		return 0;
-	is_tag_ref = !prefixcmp(path, "refs/tags/");
-
-	/* ALWAYS pack refs that were already packed or are tags */
-	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
-		return 0;
-
-	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
-	if (is_tag_ref) {
-		struct object *o = parse_object(sha1);
-		if (o->type == OBJ_TAG) {
-			o = deref_tag(o, path, 0);
-			if (o)
-				fprintf(cb->refs_file, "^%s\n",
-					sha1_to_hex(o->sha1));
-		}
-	}
-
-	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
-		int namelen = strlen(path) + 1;
-		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
-		hashcpy(n->sha1, sha1);
-		strcpy(n->name, path);
-		n->next = cb->ref_to_prune;
-		cb->ref_to_prune = n;
-	}
-	return 0;
-}
-
-/* make sure nobody touched the ref, and unlink */
-static void prune_ref(struct ref_to_prune *r)
-{
-	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
-
-	if (lock) {
-		unlink(git_path("%s", r->name));
-		unlock_ref(lock);
-	}
-}
-
-static void prune_refs(struct ref_to_prune *r)
-{
-	while (r) {
-		prune_ref(r);
-		r = r->next;
-	}
-}
-
-static struct lock_file packed;
-
-static int pack_refs(unsigned int flags)
-{
-	int fd;
-	struct pack_refs_cb_data cbdata;
-
-	memset(&cbdata, 0, sizeof(cbdata));
-	cbdata.flags = flags;
-
-	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
-	cbdata.refs_file = fdopen(fd, "w");
-	if (!cbdata.refs_file)
-		die("unable to create ref-pack file structure (%s)",
-		    strerror(errno));
-
-	/* perhaps other traits later as well */
-	fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
-
-	for_each_ref(handle_one_ref, &cbdata);
-	if (ferror(cbdata.refs_file))
-		die("failed to write ref-pack file");
-	if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
-		die("failed to write ref-pack file (%s)", strerror(errno));
-	/*
-	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
-	 * assign -1 to the lock file descriptor so that commit_lock_file()
-	 * won't try to close() it.
-	 */
-	packed.fd = -1;
-	if (commit_lock_file(&packed) < 0)
-		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
-	if (cbdata.flags & PACK_REFS_PRUNE)
-		prune_refs(cbdata.ref_to_prune);
-	return 0;
-}
+#include "pack-refs.h"
 
 static char const * const pack_refs_usage[] = {
 	"git-pack-refs [options]",
diff --git a/pack-refs.c b/pack-refs.c
new file mode 100644
index 0000000..848d311
--- /dev/null
+++ b/pack-refs.c
@@ -0,0 +1,117 @@
+#include "cache.h"
+#include "refs.h"
+#include "tag.h"
+#include "pack-refs.h"
+
+struct ref_to_prune {
+	struct ref_to_prune *next;
+	unsigned char sha1[20];
+	char name[FLEX_ARRAY];
+};
+
+struct pack_refs_cb_data {
+	unsigned int flags;
+	struct ref_to_prune *ref_to_prune;
+	FILE *refs_file;
+};
+
+static int do_not_prune(int flags)
+{
+	/* If it is already packed or if it is a symref,
+	 * do not prune it.
+	 */
+	return (flags & (REF_ISSYMREF|REF_ISPACKED));
+}
+
+static int handle_one_ref(const char *path, const unsigned char *sha1,
+			  int flags, void *cb_data)
+{
+	struct pack_refs_cb_data *cb = cb_data;
+	int is_tag_ref;
+
+	/* Do not pack the symbolic refs */
+	if ((flags & REF_ISSYMREF))
+		return 0;
+	is_tag_ref = !prefixcmp(path, "refs/tags/");
+
+	/* ALWAYS pack refs that were already packed or are tags */
+	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
+		return 0;
+
+	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	if (is_tag_ref) {
+		struct object *o = parse_object(sha1);
+		if (o->type == OBJ_TAG) {
+			o = deref_tag(o, path, 0);
+			if (o)
+				fprintf(cb->refs_file, "^%s\n",
+					sha1_to_hex(o->sha1));
+		}
+	}
+
+	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
+		int namelen = strlen(path) + 1;
+		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
+		hashcpy(n->sha1, sha1);
+		strcpy(n->name, path);
+		n->next = cb->ref_to_prune;
+		cb->ref_to_prune = n;
+	}
+	return 0;
+}
+
+/* make sure nobody touched the ref, and unlink */
+static void prune_ref(struct ref_to_prune *r)
+{
+	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
+
+	if (lock) {
+		unlink(git_path("%s", r->name));
+		unlock_ref(lock);
+	}
+}
+
+static void prune_refs(struct ref_to_prune *r)
+{
+	while (r) {
+		prune_ref(r);
+		r = r->next;
+	}
+}
+
+static struct lock_file packed;
+
+int pack_refs(unsigned int flags)
+{
+	int fd;
+	struct pack_refs_cb_data cbdata;
+
+	memset(&cbdata, 0, sizeof(cbdata));
+	cbdata.flags = flags;
+
+	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
+	cbdata.refs_file = fdopen(fd, "w");
+	if (!cbdata.refs_file)
+		die("unable to create ref-pack file structure (%s)",
+		    strerror(errno));
+
+	/* perhaps other traits later as well */
+	fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
+
+	for_each_ref(handle_one_ref, &cbdata);
+	if (ferror(cbdata.refs_file))
+		die("failed to write ref-pack file");
+	if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
+		die("failed to write ref-pack file (%s)", strerror(errno));
+	/*
+	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
+	 * assign -1 to the lock file descriptor so that commit_lock_file()
+	 * won't try to close() it.
+	 */
+	packed.fd = -1;
+	if (commit_lock_file(&packed) < 0)
+		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
+	if (cbdata.flags & PACK_REFS_PRUNE)
+		prune_refs(cbdata.ref_to_prune);
+	return 0;
+}
diff --git a/pack-refs.h b/pack-refs.h
new file mode 100644
index 0000000..518acfb
--- /dev/null
+++ b/pack-refs.h
@@ -0,0 +1,18 @@
+#ifndef PACK_REFS_H
+#define PACK_REFS_H
+
+/*
+ * Flags for controlling behaviour of pack_refs()
+ * PACK_REFS_PRUNE: Prune loose refs after packing
+ * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs
+ */
+#define PACK_REFS_PRUNE 0x0001
+#define PACK_REFS_ALL   0x0002
+
+/*
+ * Write a packed-refs file for the current repository.
+ * flags: Combination of the above PACK_REFS_* flags.
+ */
+int pack_refs(unsigned int flags);
+
+#endif /* PACK_REFS_H */
-- 
1.5.5.rc0.117.ga5237

[RFC/PATCH 2/3] Prepare testsuite for a "git clone" that packs refs

[Johan Herland]

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t5515-fetch-merge-logic.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 31c1081..4569a13 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -143,6 +143,7 @@ do
 			rm -f .git/refs/heads/*
 			rm -f .git/refs/remotes/rem/*
 			rm -f .git/refs/tags/*
+			rm -f .git/packed-refs
 			git fetch "$@" >/dev/null
 			cat .git/FETCH_HEAD
 		} >"$actual" &&
-- 
1.5.5.rc0.117.ga5237

[PATCH 3/3] Teach "git clone" to pack refs

[Johan Herland]

In repos with many refs, it is unlikely that most refs will ever change.
This fact is already exploited by "git gc" by executing "git pack-refs"
to consolidate all refs into a single file.

When cloning a repo with many refs, it does not make sense to create the
loose refs in the first place, just to have the next "git gc" consolidate
them into one file. Instead, make "git clone" create the packed refs file
immediately, and forego the loose refs completely.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-clone.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 0a9c873..01c595e 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -18,6 +18,7 @@
 #include "transport.h"
 #include "strbuf.h"
 #include "dir.h"
+#include "pack-refs.h"
 
 /*
  * Overall FIXMEs:
@@ -313,8 +314,11 @@ static struct ref *write_remote_refs(const struct ref *refs, struct refspec *ref
 	get_fetch_map(refs, tag_refspec, &tail, 0);
 
 	for (r = local_refs; r; r = r->next)
-		update_ref(reflog,
-			   r->peer_ref->name, r->old_sha1, NULL, 0, DIE_ON_ERR);
+		add_extra_ref(r->peer_ref->name, r->old_sha1, 0);
+
+	pack_refs(PACK_REFS_ALL);
+	clear_extra_refs();
+
 	return local_refs;
 }
 
-- 
1.5.5.rc0.117.ga5237

Re: [PATCH 3/3] Teach "git clone" to pack refs

[Daniel Barkalow]

On Sat, 22 Mar 2008, Johan Herland wrote:

> In repos with many refs, it is unlikely that most refs will ever change.
> This fact is already exploited by "git gc" by executing "git pack-refs"
> to consolidate all refs into a single file.
> 
> When cloning a repo with many refs, it does not make sense to create the
> loose refs in the first place, just to have the next "git gc" consolidate
> them into one file. Instead, make "git clone" create the packed refs file
> immediately, and forego the loose refs completely.

Acked-by: Daniel Barkalow <barkalow@iabervon.org>

> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>  builtin-clone.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin-clone.c b/builtin-clone.c
> index 0a9c873..01c595e 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -18,6 +18,7 @@
>  #include "transport.h"
>  #include "strbuf.h"
>  #include "dir.h"
> +#include "pack-refs.h"
>  
>  /*
>   * Overall FIXMEs:
> @@ -313,8 +314,11 @@ static struct ref *write_remote_refs(const struct ref *refs, struct refspec *ref
>  	get_fetch_map(refs, tag_refspec, &tail, 0);
>  
>  	for (r = local_refs; r; r = r->next)
> -		update_ref(reflog,
> -			   r->peer_ref->name, r->old_sha1, NULL, 0, DIE_ON_ERR);
> +		add_extra_ref(r->peer_ref->name, r->old_sha1, 0);

Wow, clever. Yes, this should work perfectly.

> +
> +	pack_refs(PACK_REFS_ALL);
> +	clear_extra_refs();
> +
>  	return local_refs;
>  }
>  
> -- 
> 1.5.5.rc0.117.ga5237
> 
> 

Re: [RFC/PATCH 0/3] Teach builtin-clone to pack refs

[Junio C Hamano]

Johan Herland <johan@herland.net> writes:

> Although most of the speedup from current "next" is achieved by the
> builtin-clone work, there is still a considerable additional improvement
> from writing all refs to a single file instead of writing one file per
> ref. I expect the performance improvement to be much bigger on platforms
> with slower filesystem (aka. Windows).

At some point, additional speedups are hidden in the noise.

Not writing reflogs is a _different_ behaviour from the previous, but I
suspect it might even be an improvement.  When you have 1000 remote
branches, probably most of them are not even active.

Re: [RFC/PATCH 0/3] Teach builtin-clone to pack refs

[Johan Herland]

On Sunday 23 March 2008, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> 
> > Although most of the speedup from current "next" is achieved by the
> > builtin-clone work, there is still a considerable additional improvement
> > from writing all refs to a single file instead of writing one file per
> > ref. I expect the performance improvement to be much bigger on platforms
> > with slower filesystem (aka. Windows).
> 
> At some point, additional speedups are hidden in the noise.

Yes, however, I bet you'll notice the difference between writing 11000 files on Windows, and writing 1.

Unfortunately, this scenario is not as far fetched as some may think: At $dayjob I'm working on converting old CVS modules with up to ~10 years of history, and some of the resulting repos have ~30000 refs (mostly build tags). Although I'll probably remove the build tags before preparing the repos that most other developers will see, I'll still have to keep the build tags around somewhere, in case devs need to refer to them. And, of course, most devs are still Windows-users. Keeping refs packed is pretty much crucial in this scenario.

> Not writing reflogs is a _different_ behaviour from the previous, but I
> suspect it might even be an improvement.  When you have 1000 remote
> branches, probably most of them are not even active.

Exactly my thinking as well. And for those few that _are_ active, you'll still of course get regular reflog entries when they _do_ change.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [RFC/PATCH 2/3] Prepare testsuite for a "git clone" that packs refs

[Daniel Barkalow]

On Sat, 22 Mar 2008, Johan Herland wrote:

> diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
> index 31c1081..4569a13 100755
> --- a/t/t5515-fetch-merge-logic.sh
> +++ b/t/t5515-fetch-merge-logic.sh
> @@ -143,6 +143,7 @@ do
>  			rm -f .git/refs/heads/*
>  			rm -f .git/refs/remotes/rem/*
>  			rm -f .git/refs/tags/*
> +			rm -f .git/packed-refs
>  			git fetch "$@" >/dev/null
>  			cat .git/FETCH_HEAD
>  		} >"$actual" &&

I was just looking over this again, and it doesn't actually work, because 
it removes the copies of refs/remotes/origin/*, which the tests are 
expecting to find.

	-Daniel
*This .sig left intentionally blank*

Re: [RFC/PATCH 2/3] Prepare testsuite for a "git clone" that packs refs

[Johan Herland]

On Monday 14 April 2008, Daniel Barkalow wrote:
> On Sat, 22 Mar 2008, Johan Herland wrote:
> 
> > diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
> > index 31c1081..4569a13 100755
> > --- a/t/t5515-fetch-merge-logic.sh
> > +++ b/t/t5515-fetch-merge-logic.sh
> > @@ -143,6 +143,7 @@ do
> >  			rm -f .git/refs/heads/*
> >  			rm -f .git/refs/remotes/rem/*
> >  			rm -f .git/refs/tags/*
> > +			rm -f .git/packed-refs
> >  			git fetch "$@" >/dev/null
> >  			cat .git/FETCH_HEAD
> >  		} >"$actual" &&
> 
> I was just looking over this again, and it doesn't actually work, because 
> it removes the copies of refs/remotes/origin/*, which the tests are 
> expecting to find.

Yes, I also found that (after rebasing on current 'next'). :)

Will shortly post a new series making builtin-clone do packed refs,
suitable for current 'next'.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

[PATCH 1/4] Incorporate fetched packs in future object traversal

[Johan Herland]

Immediately after fetching a pack, we should call reprepare_packed_git() to
make sure the objects in the pack are reachable. Otherwise, we will fail to
look up objects that are present only in the fetched pack.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-fetch-pack.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 65350ca..309ae03 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -820,5 +820,6 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 		}
 	}
 
+	reprepare_packed_git();
 	return ref_cpy;
 }
-- 
1.5.5.159.g8c84b

[PATCH 2/4] Move pack_refs() and friends into libgit

[Johan Herland]

This moves pack_refs() and underlying functionality into the library,
to make pack-refs functionality easily available to all git programs.

Most of builtin-pack-refs.c has been moved verbatim into a new file
pack-refs.c that is compiled into libgit.a. A corresponding header
file, pack-refs.h, has also been added, declaring pack_refs() and
the #defines associated with the flags parameter to pack_refs().

This patch introduces no other changes in functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Makefile            |    2 +
 builtin-pack-refs.c |  121 +--------------------------------------------------
 pack-refs.c         |  117 +++++++++++++++++++++++++++++++++++++++++++++++++
 pack-refs.h         |   18 ++++++++
 4 files changed, 138 insertions(+), 120 deletions(-)
 create mode 100644 pack-refs.c
 create mode 100644 pack-refs.h

diff --git a/Makefile b/Makefile
index 854527d..6a1c23c 100644
--- a/Makefile
+++ b/Makefile
@@ -353,6 +353,7 @@ LIB_H += log-tree.h
 LIB_H += mailmap.h
 LIB_H += object.h
 LIB_H += pack.h
+LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
@@ -425,6 +426,7 @@ LIB_OBJS += merge-file.o
 LIB_OBJS += name-hash.o
 LIB_OBJS += object.o
 LIB_OBJS += pack-check.o
+LIB_OBJS += pack-refs.o
 LIB_OBJS += pack-revindex.o
 LIB_OBJS += pack-write.o
 LIB_OBJS += pager.o
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 1aaa76d..ff90aef 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -1,125 +1,6 @@
-#include "builtin.h"
 #include "cache.h"
-#include "refs.h"
-#include "object.h"
-#include "tag.h"
 #include "parse-options.h"
-
-struct ref_to_prune {
-	struct ref_to_prune *next;
-	unsigned char sha1[20];
-	char name[FLEX_ARRAY];
-};
-
-#define PACK_REFS_PRUNE	0x0001
-#define PACK_REFS_ALL	0x0002
-
-struct pack_refs_cb_data {
-	unsigned int flags;
-	struct ref_to_prune *ref_to_prune;
-	FILE *refs_file;
-};
-
-static int do_not_prune(int flags)
-{
-	/* If it is already packed or if it is a symref,
-	 * do not prune it.
-	 */
-	return (flags & (REF_ISSYMREF|REF_ISPACKED));
-}
-
-static int handle_one_ref(const char *path, const unsigned char *sha1,
-			  int flags, void *cb_data)
-{
-	struct pack_refs_cb_data *cb = cb_data;
-	int is_tag_ref;
-
-	/* Do not pack the symbolic refs */
-	if ((flags & REF_ISSYMREF))
-		return 0;
-	is_tag_ref = !prefixcmp(path, "refs/tags/");
-
-	/* ALWAYS pack refs that were already packed or are tags */
-	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
-		return 0;
-
-	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
-	if (is_tag_ref) {
-		struct object *o = parse_object(sha1);
-		if (o->type == OBJ_TAG) {
-			o = deref_tag(o, path, 0);
-			if (o)
-				fprintf(cb->refs_file, "^%s\n",
-					sha1_to_hex(o->sha1));
-		}
-	}
-
-	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
-		int namelen = strlen(path) + 1;
-		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
-		hashcpy(n->sha1, sha1);
-		strcpy(n->name, path);
-		n->next = cb->ref_to_prune;
-		cb->ref_to_prune = n;
-	}
-	return 0;
-}
-
-/* make sure nobody touched the ref, and unlink */
-static void prune_ref(struct ref_to_prune *r)
-{
-	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
-
-	if (lock) {
-		unlink(git_path("%s", r->name));
-		unlock_ref(lock);
-	}
-}
-
-static void prune_refs(struct ref_to_prune *r)
-{
-	while (r) {
-		prune_ref(r);
-		r = r->next;
-	}
-}
-
-static struct lock_file packed;
-
-static int pack_refs(unsigned int flags)
-{
-	int fd;
-	struct pack_refs_cb_data cbdata;
-
-	memset(&cbdata, 0, sizeof(cbdata));
-	cbdata.flags = flags;
-
-	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
-	cbdata.refs_file = fdopen(fd, "w");
-	if (!cbdata.refs_file)
-		die("unable to create ref-pack file structure (%s)",
-		    strerror(errno));
-
-	/* perhaps other traits later as well */
-	fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
-
-	for_each_ref(handle_one_ref, &cbdata);
-	if (ferror(cbdata.refs_file))
-		die("failed to write ref-pack file");
-	if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
-		die("failed to write ref-pack file (%s)", strerror(errno));
-	/*
-	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
-	 * assign -1 to the lock file descriptor so that commit_lock_file()
-	 * won't try to close() it.
-	 */
-	packed.fd = -1;
-	if (commit_lock_file(&packed) < 0)
-		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
-	if (cbdata.flags & PACK_REFS_PRUNE)
-		prune_refs(cbdata.ref_to_prune);
-	return 0;
-}
+#include "pack-refs.h"
 
 static char const * const pack_refs_usage[] = {
 	"git-pack-refs [options]",
diff --git a/pack-refs.c b/pack-refs.c
new file mode 100644
index 0000000..848d311
--- /dev/null
+++ b/pack-refs.c
@@ -0,0 +1,117 @@
+#include "cache.h"
+#include "refs.h"
+#include "tag.h"
+#include "pack-refs.h"
+
+struct ref_to_prune {
+	struct ref_to_prune *next;
+	unsigned char sha1[20];
+	char name[FLEX_ARRAY];
+};
+
+struct pack_refs_cb_data {
+	unsigned int flags;
+	struct ref_to_prune *ref_to_prune;
+	FILE *refs_file;
+};
+
+static int do_not_prune(int flags)
+{
+	/* If it is already packed or if it is a symref,
+	 * do not prune it.
+	 */
+	return (flags & (REF_ISSYMREF|REF_ISPACKED));
+}
+
+static int handle_one_ref(const char *path, const unsigned char *sha1,
+			  int flags, void *cb_data)
+{
+	struct pack_refs_cb_data *cb = cb_data;
+	int is_tag_ref;
+
+	/* Do not pack the symbolic refs */
+	if ((flags & REF_ISSYMREF))
+		return 0;
+	is_tag_ref = !prefixcmp(path, "refs/tags/");
+
+	/* ALWAYS pack refs that were already packed or are tags */
+	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
+		return 0;
+
+	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	if (is_tag_ref) {
+		struct object *o = parse_object(sha1);
+		if (o->type == OBJ_TAG) {
+			o = deref_tag(o, path, 0);
+			if (o)
+				fprintf(cb->refs_file, "^%s\n",
+					sha1_to_hex(o->sha1));
+		}
+	}
+
+	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
+		int namelen = strlen(path) + 1;
+		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
+		hashcpy(n->sha1, sha1);
+		strcpy(n->name, path);
+		n->next = cb->ref_to_prune;
+		cb->ref_to_prune = n;
+	}
+	return 0;
+}
+
+/* make sure nobody touched the ref, and unlink */
+static void prune_ref(struct ref_to_prune *r)
+{
+	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
+
+	if (lock) {
+		unlink(git_path("%s", r->name));
+		unlock_ref(lock);
+	}
+}
+
+static void prune_refs(struct ref_to_prune *r)
+{
+	while (r) {
+		prune_ref(r);
+		r = r->next;
+	}
+}
+
+static struct lock_file packed;
+
+int pack_refs(unsigned int flags)
+{
+	int fd;
+	struct pack_refs_cb_data cbdata;
+
+	memset(&cbdata, 0, sizeof(cbdata));
+	cbdata.flags = flags;
+
+	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
+	cbdata.refs_file = fdopen(fd, "w");
+	if (!cbdata.refs_file)
+		die("unable to create ref-pack file structure (%s)",
+		    strerror(errno));
+
+	/* perhaps other traits later as well */
+	fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
+
+	for_each_ref(handle_one_ref, &cbdata);
+	if (ferror(cbdata.refs_file))
+		die("failed to write ref-pack file");
+	if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
+		die("failed to write ref-pack file (%s)", strerror(errno));
+	/*
+	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
+	 * assign -1 to the lock file descriptor so that commit_lock_file()
+	 * won't try to close() it.
+	 */
+	packed.fd = -1;
+	if (commit_lock_file(&packed) < 0)
+		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
+	if (cbdata.flags & PACK_REFS_PRUNE)
+		prune_refs(cbdata.ref_to_prune);
+	return 0;
+}
diff --git a/pack-refs.h b/pack-refs.h
new file mode 100644
index 0000000..518acfb
--- /dev/null
+++ b/pack-refs.h
@@ -0,0 +1,18 @@
+#ifndef PACK_REFS_H
+#define PACK_REFS_H
+
+/*
+ * Flags for controlling behaviour of pack_refs()
+ * PACK_REFS_PRUNE: Prune loose refs after packing
+ * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs
+ */
+#define PACK_REFS_PRUNE 0x0001
+#define PACK_REFS_ALL   0x0002
+
+/*
+ * Write a packed-refs file for the current repository.
+ * flags: Combination of the above PACK_REFS_* flags.
+ */
+int pack_refs(unsigned int flags);
+
+#endif /* PACK_REFS_H */
-- 
1.5.5.159.g8c84b

[PATCH 3/4] Prepare testsuite for a "git clone" that packs refs

[Johan Herland]

t5515-fetch-merge-logic removes many, but not all, refs between each test.
This is done by removing the corresponding refs/foo/* files in the .git/refs
hierarchy. However, once "git clone" starts producing packed refs, these refs
will no longer be in the .git/refs hierarchy, but rather listed in
.git/packed-refs. This patch therefore teaches t5515-fetch-merge-logic to also
remove the refs in question from the packed-refs file.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t5515-fetch-merge-logic.sh |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 65c3774..8a8c35c 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -145,6 +145,25 @@ do
 			rm -f .git/refs/heads/*
 			rm -f .git/refs/remotes/rem/*
 			rm -f .git/refs/tags/*
+			cat .git/packed-refs | \
+			while read sha1 refname
+			do
+				case "$sha1" in
+				^*) # remove peeled tags
+					;;
+				*)
+					case "$refname" in
+					refs/heads/*|\
+					refs/remotes/rem/*|\
+					refs/tags/*) # remove same as above
+						;;
+					*) # keep everything else
+						echo "$sha1 $refname"
+						;;
+					esac
+				esac
+			done > .git/packed-refs.new
+			mv .git/packed-refs.new .git/packed-refs
 			git fetch "$@" >/dev/null
 			cat .git/FETCH_HEAD
 		} >"$actual_f" &&
-- 
1.5.5.159.g8c84b

[PATCH 4/4] Teach "git clone" to pack refs

[Johan Herland]

In repos with many refs, it is unlikely that most refs will ever change.
This fact is already exploited by "git gc" by executing "git pack-refs"
to consolidate all refs into a single file.

When cloning a repo with many refs, it does not make sense to create the
loose refs in the first place, just to have the next "git gc" consolidate
them into one file. Instead, make "git clone" create the packed refs file
immediately, and forego the loose refs completely.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-clone.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 0a9c873..01c595e 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -18,6 +18,7 @@
 #include "transport.h"
 #include "strbuf.h"
 #include "dir.h"
+#include "pack-refs.h"
 
 /*
  * Overall FIXMEs:
@@ -313,8 +314,11 @@ static struct ref *write_remote_refs(const struct ref *refs, struct refspec *ref
 	get_fetch_map(refs, tag_refspec, &tail, 0);
 
 	for (r = local_refs; r; r = r->next)
-		update_ref(reflog,
-			   r->peer_ref->name, r->old_sha1, NULL, 0, DIE_ON_ERR);
+		add_extra_ref(r->peer_ref->name, r->old_sha1, 0);
+
+	pack_refs(PACK_REFS_ALL);
+	clear_extra_refs();
+
 	return local_refs;
 }
 
-- 
1.5.5.159.g8c84b

[PATCH 2/4] Move pack_refs() and friends into libgit

[Johan Herland]

This moves pack_refs() and underlying functionality into the library,
to make pack-refs functionality easily available to all git programs.

Most of builtin-pack-refs.c has been moved verbatim into a new file
pack-refs.c that is compiled into libgit.a. A corresponding header
file, pack-refs.h, has also been added, declaring pack_refs() and
the #defines associated with the flags parameter to pack_refs().

This patch introduces no other changes in functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Makefile            |    2 +
 builtin-pack-refs.c |  121 +--------------------------------------------------
 pack-refs.c         |  117 +++++++++++++++++++++++++++++++++++++++++++++++++
 pack-refs.h         |   18 ++++++++
 4 files changed, 138 insertions(+), 120 deletions(-)
 create mode 100644 pack-refs.c
 create mode 100644 pack-refs.h

diff --git a/Makefile b/Makefile
index 1937507..4a78388 100644
--- a/Makefile
+++ b/Makefile
@@ -354,6 +354,7 @@ LIB_H += log-tree.h
 LIB_H += mailmap.h
 LIB_H += object.h
 LIB_H += pack.h
+LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
@@ -429,6 +430,7 @@ LIB_OBJS += merge-file.o
 LIB_OBJS += name-hash.o
 LIB_OBJS += object.o
 LIB_OBJS += pack-check.o
+LIB_OBJS += pack-refs.o
 LIB_OBJS += pack-revindex.o
 LIB_OBJS += pack-write.o
 LIB_OBJS += pager.o
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 1aaa76d..ff90aef 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -1,125 +1,6 @@
-#include "builtin.h"
 #include "cache.h"
-#include "refs.h"
-#include "object.h"
-#include "tag.h"
 #include "parse-options.h"
-
-struct ref_to_prune {
-	struct ref_to_prune *next;
-	unsigned char sha1[20];
-	char name[FLEX_ARRAY];
-};
-
-#define PACK_REFS_PRUNE	0x0001
-#define PACK_REFS_ALL	0x0002
-
-struct pack_refs_cb_data {
-	unsigned int flags;
-	struct ref_to_prune *ref_to_prune;
-	FILE *refs_file;
-};
-
-static int do_not_prune(int flags)
-{
-	/* If it is already packed or if it is a symref,
-	 * do not prune it.
-	 */
-	return (flags & (REF_ISSYMREF|REF_ISPACKED));
-}
-
-static int handle_one_ref(const char *path, const unsigned char *sha1,
-			  int flags, void *cb_data)
-{
-	struct pack_refs_cb_data *cb = cb_data;
-	int is_tag_ref;
-
-	/* Do not pack the symbolic refs */
-	if ((flags & REF_ISSYMREF))
-		return 0;
-	is_tag_ref = !prefixcmp(path, "refs/tags/");
-
-	/* ALWAYS pack refs that were already packed or are tags */
-	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
-		return 0;
-
-	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
-	if (is_tag_ref) {
-		struct object *o = parse_object(sha1);
-		if (o->type == OBJ_TAG) {
-			o = deref_tag(o, path, 0);
-			if (o)
-				fprintf(cb->refs_file, "^%s\n",
-					sha1_to_hex(o->sha1));
-		}
-	}
-
-	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
-		int namelen = strlen(path) + 1;
-		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
-		hashcpy(n->sha1, sha1);
-		strcpy(n->name, path);
-		n->next = cb->ref_to_prune;
-		cb->ref_to_prune = n;
-	}
-	return 0;
-}
-
-/* make sure nobody touched the ref, and unlink */
-static void prune_ref(struct ref_to_prune *r)
-{
-	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
-
-	if (lock) {
-		unlink(git_path("%s", r->name));
-		unlock_ref(lock);
-	}
-}
-
-static void prune_refs(struct ref_to_prune *r)
-{
-	while (r) {
-		prune_ref(r);
-		r = r->next;
-	}
-}
-
-static struct lock_file packed;
-
-static int pack_refs(unsigned int flags)
-{
-	int fd;
-	struct pack_refs_cb_data cbdata;
-
-	memset(&cbdata, 0, sizeof(cbdata));
-	cbdata.flags = flags;
-
-	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
-	cbdata.refs_file = fdopen(fd, "w");
-	if (!cbdata.refs_file)
-		die("unable to create ref-pack file structure (%s)",
-		    strerror(errno));
-
-	/* perhaps other traits later as well */
-	fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
-
-	for_each_ref(handle_one_ref, &cbdata);
-	if (ferror(cbdata.refs_file))
-		die("failed to write ref-pack file");
-	if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
-		die("failed to write ref-pack file (%s)", strerror(errno));
-	/*
-	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
-	 * assign -1 to the lock file descriptor so that commit_lock_file()
-	 * won't try to close() it.
-	 */
-	packed.fd = -1;
-	if (commit_lock_file(&packed) < 0)
-		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
-	if (cbdata.flags & PACK_REFS_PRUNE)
-		prune_refs(cbdata.ref_to_prune);
-	return 0;
-}
+#include "pack-refs.h"
 
 static char const * const pack_refs_usage[] = {
 	"git-pack-refs [options]",
diff --git a/pack-refs.c b/pack-refs.c
new file mode 100644
index 0000000..848d311
--- /dev/null
+++ b/pack-refs.c
@@ -0,0 +1,117 @@
+#include "cache.h"
+#include "refs.h"
+#include "tag.h"
+#include "pack-refs.h"
+
+struct ref_to_prune {
+	struct ref_to_prune *next;
+	unsigned char sha1[20];
+	char name[FLEX_ARRAY];
+};
+
+struct pack_refs_cb_data {
+	unsigned int flags;
+	struct ref_to_prune *ref_to_prune;
+	FILE *refs_file;
+};
+
+static int do_not_prune(int flags)
+{
+	/* If it is already packed or if it is a symref,
+	 * do not prune it.
+	 */
+	return (flags & (REF_ISSYMREF|REF_ISPACKED));
+}
+
+static int handle_one_ref(const char *path, const unsigned char *sha1,
+			  int flags, void *cb_data)
+{
+	struct pack_refs_cb_data *cb = cb_data;
+	int is_tag_ref;
+
+	/* Do not pack the symbolic refs */
+	if ((flags & REF_ISSYMREF))
+		return 0;
+	is_tag_ref = !prefixcmp(path, "refs/tags/");
+
+	/* ALWAYS pack refs that were already packed or are tags */
+	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
+		return 0;
+
+	fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	if (is_tag_ref) {
+		struct object *o = parse_object(sha1);
+		if (o->type == OBJ_TAG) {
+			o = deref_tag(o, path, 0);
+			if (o)
+				fprintf(cb->refs_file, "^%s\n",
+					sha1_to_hex(o->sha1));
+		}
+	}
+
+	if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
+		int namelen = strlen(path) + 1;
+		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
+		hashcpy(n->sha1, sha1);
+		strcpy(n->name, path);
+		n->next = cb->ref_to_prune;
+		cb->ref_to_prune = n;
+	}
+	return 0;
+}
+
+/* make sure nobody touched the ref, and unlink */
+static void prune_ref(struct ref_to_prune *r)
+{
+	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);
+
+	if (lock) {
+		unlink(git_path("%s", r->name));
+		unlock_ref(lock);
+	}
+}
+
+static void prune_refs(struct ref_to_prune *r)
+{
+	while (r) {
+		prune_ref(r);
+		r = r->next;
+	}
+}
+
+static struct lock_file packed;
+
+int pack_refs(unsigned int flags)
+{
+	int fd;
+	struct pack_refs_cb_data cbdata;
+
+	memset(&cbdata, 0, sizeof(cbdata));
+	cbdata.flags = flags;
+
+	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
+	cbdata.refs_file = fdopen(fd, "w");
+	if (!cbdata.refs_file)
+		die("unable to create ref-pack file structure (%s)",
+		    strerror(errno));
+
+	/* perhaps other traits later as well */
+	fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
+
+	for_each_ref(handle_one_ref, &cbdata);
+	if (ferror(cbdata.refs_file))
+		die("failed to write ref-pack file");
+	if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
+		die("failed to write ref-pack file (%s)", strerror(errno));
+	/*
+	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
+	 * assign -1 to the lock file descriptor so that commit_lock_file()
+	 * won't try to close() it.
+	 */
+	packed.fd = -1;
+	if (commit_lock_file(&packed) < 0)
+		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
+	if (cbdata.flags & PACK_REFS_PRUNE)
+		prune_refs(cbdata.ref_to_prune);
+	return 0;
+}
diff --git a/pack-refs.h b/pack-refs.h
new file mode 100644
index 0000000..518acfb
--- /dev/null
+++ b/pack-refs.h
@@ -0,0 +1,18 @@
+#ifndef PACK_REFS_H
+#define PACK_REFS_H
+
+/*
+ * Flags for controlling behaviour of pack_refs()
+ * PACK_REFS_PRUNE: Prune loose refs after packing
+ * PACK_REFS_ALL:   Pack _all_ refs, not just tags and already packed refs
+ */
+#define PACK_REFS_PRUNE 0x0001
+#define PACK_REFS_ALL   0x0002
+
+/*
+ * Write a packed-refs file for the current repository.
+ * flags: Combination of the above PACK_REFS_* flags.
+ */
+int pack_refs(unsigned int flags);
+
+#endif /* PACK_REFS_H */
-- 
1.5.6.rc2.128.gf64ae

Re: [PATCH 2/4] Move pack_refs() and friends into libgit

[Jeff King]

On Sun, Jun 15, 2008 at 04:05:06PM +0200, Johan Herland wrote:

> ---
>  Makefile            |    2 +
>  builtin-pack-refs.c |  121 +-----------------------------------------------
>  pack-refs.c         |  117 ++++++++++++++++++++++++++++++++++++++++++++++
>  pack-refs.h         |   18 ++++++++
>  4 files changed, 138 insertions(+), 120 deletions(-)
>  create mode 100644 pack-refs.c
>  create mode 100644 pack-refs.h

This patch would have been a lot easier to read, btw, if it had been
generated with '-C' (then patch to pack-refs.c is based on
builtin-pack-refs instead of /dev/null).

-Peff