Re: rs/commit-list-sort-in-batch

On Sat, Apr 07, 2012 at 03:27:24AM -0400, Jeff King wrote:

On Fri, Apr 06, 2012 at 02:02:56PM -0700, Junio C Hamano wrote:

quoted

* rs/commit-list-sort-in-batch (2012-04-02) 3 commits
 - revision: insert unsorted, then sort in prepare_revision_walk()
 - commit: use mergesort() in commit_list_sort_by_date()
 - add mergesort() for linked lists

Looks like you applied these patches right on v1.7.9. However, they need
Thomas's 5e8617f to compile, which comes later. There's no textual
conflict, but test-mergesort.c need strbuf_getwholeline_fd, which is
made public by 5e8617f.

By the way, building on top of René's patches[1], I was able to drop the
time of a particular fetch into a repository with many refs from:

    real    0m2.081s
    user    0m2.180s
    sys     0m0.712s

to:

    real    0m1.784s
    user    0m1.936s
    sys     0m0.652s

with this patch:

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 7124c4b..5d80e65 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c

@@ -503,7 +503,7 @@ static int mark_complete(const char *refname, const unsigned char *sha1, int fla
 		struct commit *commit = (struct commit *)o;
 		if (!(commit->object.flags & COMPLETE)) {
 			commit->object.flags |= COMPLETE;
-			commit_list_insert_by_date(commit, &complete);
+			commit_list_insert(commit, &complete);
 		}
 	}
 	return 0;

@@ -615,6 +615,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
 	if (!args.depth) {
 		for_each_ref(mark_complete, NULL);
 		for_each_alternate_ref(mark_alternate_complete, NULL);
+		commit_list_sort_by_date(&complete);
 		if (cutoff)
 			mark_recent_complete_commits(cutoff);
 	}

I'm not sure if that vindicates the patches, or implies that these
things are lurking everywhere and that we should really move to a better
data structure so we don't have to fix them all manually. :)

-Peff

[1] Actually, it was René's patches merged into v1.7.10. My ccdc603
    (parse_object: try internal cache before reading object db,
    2012-01-05) makes such a huge difference it is not worth timing
    without it.