Merge branch 'rs/mergesort'

Make our mergesort implementation type-safe.

* rs/mergesort:
  mergesort: remove llist_mergesort()
  packfile: use DEFINE_LIST_SORT
  fetch-pack: use DEFINE_LIST_SORT
  commit: use DEFINE_LIST_SORT
  blame: use DEFINE_LIST_SORT
  test-mergesort: use DEFINE_LIST_SORT
  test-mergesort: use DEFINE_LIST_SORT_DEBUG
  mergesort: add macros for typed sort of linked lists
  mergesort: tighten merge loop
  mergesort: unify ranks loops

Author: gitster

Makefile

@@ -992,7 +992,6 @@ LIB_OBJS += merge-ort.o
 LIB_OBJS += merge-ort-wrappers.o
 LIB_OBJS += merge-recursive.o
 LIB_OBJS += merge.o
-LIB_OBJS += mergesort.o
 LIB_OBJS += midx.o
 LIB_OBJS += name-hash.o
 LIB_OBJS += negotiator/default.o

blame.c

@@ -1098,30 +1098,22 @@ static struct blame_entry *blame_merge(struct blame_entry *list1,
 	}
 }
 
-static void *get_next_blame(const void *p)
-{
-	return ((struct blame_entry *)p)->next;
-}
-
-static void set_next_blame(void *p1, void *p2)
-{
-	((struct blame_entry *)p1)->next = p2;
-}
+DEFINE_LIST_SORT(static, sort_blame_entries, struct blame_entry, next);
 
 /*
  * Final image line numbers are all different, so we don't need a
  * three-way comparison here.
  */
 
-static int compare_blame_final(const void *p1, const void *p2)
+static int compare_blame_final(const struct blame_entry *e1,
+			       const struct blame_entry *e2)
 {
-	return ((struct blame_entry *)p1)->lno > ((struct blame_entry *)p2)->lno
-		? 1 : -1;
+	return e1->lno > e2->lno ? 1 : -1;
 }
 
-static int compare_blame_suspect(const void *p1, const void *p2)
+static int compare_blame_suspect(const struct blame_entry *s1,
+				 const struct blame_entry *s2)
 {
-	const struct blame_entry *s1 = p1, *s2 = p2;
 	/*
 	 * to allow for collating suspects, we sort according to the
 	 * respective pointer value as the primary sorting criterion.
@@ -1138,8 +1130,7 @@ static int compare_blame_suspect(const void *p1, const void *p2)
 
 void blame_sort_final(struct blame_scoreboard *sb)
 {
-	sb->ent = llist_mergesort(sb->ent, get_next_blame, set_next_blame,
-				  compare_blame_final);
+	sort_blame_entries(&sb->ent, compare_blame_final);
 }
 
 static int compare_commits_by_reverse_commit_date(const void *a,
@@ -1964,9 +1955,7 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
 		    parent, target, 0);
 	*d.dstq = NULL;
 	if (ignore_diffs)
-		newdest = llist_mergesort(newdest, get_next_blame,
-					  set_next_blame,
-					  compare_blame_suspect);
+		sort_blame_entries(&newdest, compare_blame_suspect);
 	queue_blames(sb, parent, newdest);
 
 	return;
@@ -2383,8 +2372,7 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit, int reve
  */
 static void distribute_blame(struct blame_scoreboard *sb, struct blame_entry *blamed)
 {
-	blamed = llist_mergesort(blamed, get_next_blame, set_next_blame,
-				 compare_blame_suspect);
+	sort_blame_entries(&blamed, compare_blame_suspect);
 	while (blamed)
 	{
 		struct blame_origin *porigin = blamed->suspect;

commit.c

@@ -642,31 +642,23 @@ struct commit_list * commit_list_insert_by_date(struct commit *item, struct comm
 	return commit_list_insert(item, pp);
 }
 
-static int commit_list_compare_by_date(const void *a, const void *b)
+static int commit_list_compare_by_date(const struct commit_list *a,
+				       const struct commit_list *b)
 {
-	timestamp_t a_date = ((const struct commit_list *)a)->item->date;
-	timestamp_t b_date = ((const struct commit_list *)b)->item->date;
+	timestamp_t a_date = a->item->date;
+	timestamp_t b_date = b->item->date;
 	if (a_date < b_date)
 		return 1;
 	if (a_date > b_date)
 		return -1;
 	return 0;
 }
 
-static void *commit_list_get_next(const void *a)
-{
-	return ((const struct commit_list *)a)->next;
-}
-
-static void commit_list_set_next(void *a, void *next)
-{
-	((struct commit_list *)a)->next = next;
-}
+DEFINE_LIST_SORT(static, commit_list_sort, struct commit_list, next);
 
 void commit_list_sort_by_date(struct commit_list **list)
 {
-	*list = llist_mergesort(*list, commit_list_get_next, commit_list_set_next,
-				commit_list_compare_by_date);
+	commit_list_sort(list, commit_list_compare_by_date);
 }
 
 struct commit *pop_most_recent_commit(struct commit_list **list,

fetch-pack.c

@@ -26,6 +26,7 @@
 #include "commit-reach.h"
 #include "commit-graph.h"
 #include "sigchain.h"
+#include "mergesort.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -1025,6 +1026,13 @@ static int get_pack(struct fetch_pack_args *args,
 	return 0;
 }
 
+static int ref_compare_name(const struct ref *a, const struct ref *b)
+{
+	return strcmp(a->name, b->name);
+}
+
+DEFINE_LIST_SORT(static, sort_ref_list, struct ref, next);
+
 static int cmp_ref_by_name(const void *a_, const void *b_)
 {
 	const struct ref *a = *((const struct ref **)a_);

mergesort.c

@@ -1,17 +1,105 @@
 #ifndef MERGESORT_H
 #define MERGESORT_H
 
+/* Combine two sorted lists.  Take from `list` on equality. */
+#define DEFINE_LIST_MERGE_INTERNAL(name, type)				\
+static type *name##__merge(type *list, type *other,			\
+			   int (*compare_fn)(const type *, const type *))\
+{									\
+	type *result = list, *tail;					\
+	int prefer_list = compare_fn(list, other) <= 0;			\
+									\
+	if (!prefer_list) {						\
+		result = other;						\
+		SWAP(list, other);					\
+	}								\
+	for (;;) {							\
+		do {							\
+			tail = list;					\
+			list = name##__get_next(list);			\
+			if (!list) {					\
+				name##__set_next(tail, other);		\
+				return result;				\
+			}						\
+		} while (compare_fn(list, other) < prefer_list);	\
+		name##__set_next(tail, other);				\
+		prefer_list ^= 1;					\
+		SWAP(list, other);					\
+	}								\
+}
+
 /*
- * Sort linked list in place.
- * - get_next_fn() returns the next element given an element of a linked list.
- * - set_next_fn() takes two elements A and B, and makes B the "next" element
- *   of A on the list.
- * - compare_fn() takes two elements A and B, and returns negative, 0, positive
- *   as the same sign as "subtracting" B from A.
+ * Perform an iterative mergesort using an array of sublists.
+ *
+ * n is the number of items.
+ * ranks[i] is undefined if n & 2^i == 0, and assumed empty.
+ * ranks[i] contains a sublist of length 2^i otherwise.
+ *
+ * The number of bits in a void pointer limits the number of objects
+ * that can be created, and thus the number of array elements necessary
+ * to be able to sort any valid list.
+ *
+ * Adding an item to this array is like incrementing a binary number;
+ * positional values for set bits correspond to sublist lengths.
  */
-void *llist_mergesort(void *list,
-		      void *(*get_next_fn)(const void *),
-		      void (*set_next_fn)(void *, void *),
-		      int (*compare_fn)(const void *, const void *));
+#define DEFINE_LIST_SORT_INTERNAL(scope, name, type)			\
+scope void name(type **listp,						\
+		int (*compare_fn)(const type *, const type *))		\
+{									\
+	type *list = *listp;						\
+	type *ranks[bitsizeof(type *)];					\
+	size_t n = 0;							\
+									\
+	if (!list)							\
+		return;							\
+									\
+	for (;;) {							\
+		int i;							\
+		size_t m;						\
+		type *next = name##__get_next(list);			\
+		if (next)						\
+			name##__set_next(list, NULL);			\
+		for (i = 0, m = n;; i++, m >>= 1) {			\
+			if (m & 1) {					\
+				list = name##__merge(ranks[i], list,	\
+						    compare_fn);	\
+			} else if (next) {				\
+				break;					\
+			} else if (!m) {				\
+				*listp = list;				\
+				return;					\
+			}						\
+		}							\
+		n++;							\
+		ranks[i] = list;					\
+		list = next;						\
+	}								\
+}
+
+#define DECLARE_LIST_SORT(scope, name, type)			\
+scope void name(type **listp,					\
+		int (*compare_fn)(const type *, const type *))
+
+#define DEFINE_LIST_SORT_DEBUG(scope, name, type, next_member,	\
+			       on_get_next, on_set_next)	\
+								\
+static inline type *name##__get_next(const type *elem)		\
+{								\
+	on_get_next;						\
+	return elem->next_member;				\
+}								\
+								\
+static inline void name##__set_next(type *elem, type *next)	\
+{								\
+	on_set_next;						\
+	elem->next_member = next;				\
+}								\
+								\
+DEFINE_LIST_MERGE_INTERNAL(name, type)				\
+DEFINE_LIST_SORT_INTERNAL(scope, name, type)			\
+DECLARE_LIST_SORT(scope, name, type)
+
+#define DEFINE_LIST_SORT(scope, name, type, next_member) \
+DEFINE_LIST_SORT_DEBUG(scope, name, type, next_member, (void)0, (void)0)
 
 #endif

mergesort.h

@@ -941,20 +941,10 @@ unsigned long repo_approximate_object_count(struct repository *r)
 	return r->objects->approximate_object_count;
 }
 
-static void *get_next_packed_git(const void *p)
-{
-	return ((const struct packed_git *)p)->next;
-}
-
-static void set_next_packed_git(void *p, void *next)
-{
-	((struct packed_git *)p)->next = next;
-}
+DEFINE_LIST_SORT(static, sort_packs, struct packed_git, next);
 
-static int sort_pack(const void *a_, const void *b_)
+static int sort_pack(const struct packed_git *a, const struct packed_git *b)
 {
-	const struct packed_git *a = a_;
-	const struct packed_git *b = b_;
 	int st;
 
 	/*
@@ -981,9 +971,7 @@ static int sort_pack(const void *a_, const void *b_)
 
 static void rearrange_packed_git(struct repository *r)
 {
-	r->objects->packed_git = llist_mergesort(
-		r->objects->packed_git, get_next_packed_git,
-		set_next_packed_git, sort_pack);
+	sort_packs(&r->objects->packed_git, sort_pack);
 }
 
 static void prepare_packed_git_mru(struct repository *r)