revision: priority queue for streaming walks + walk_ops vtable

builtin/pack-objects.c

@@ -4265,6 +4265,7 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs
 	traverse_commit_list(&revs, show_cruft_commit, show_cruft_object, NULL);
 
 	stop_progress(&progress_state);
+	release_revisions(&revs);
 }
 
 static void read_cruft_objects(void)

commit.c

@@ -729,19 +729,6 @@ void commit_list_free(struct commit_list *list)
 		pop_commit(&list);
 }
 
-struct commit_list * commit_list_insert_by_date(struct commit *item, struct commit_list **list)
-{
-	struct commit_list **pp = list;
-	struct commit_list *p;
-	while ((p = *pp) != NULL) {
-		if (p->item->date < item->date) {
-			break;
-		}
-		pp = &p->next;
-	}
-	return commit_list_insert(item, pp);
-}
-
 static int commit_list_compare_by_date(const struct commit_list *a,
 				       const struct commit_list *b)
 {

commit.h

@@ -191,8 +191,6 @@ int commit_list_contains(struct commit *item,
 struct commit_list **commit_list_append(struct commit *commit,
 					struct commit_list **next);
 unsigned commit_list_count(const struct commit_list *l);
-struct commit_list *commit_list_insert_by_date(struct commit *item,
-				    struct commit_list **list);
 void commit_list_sort_by_date(struct commit_list **list);
 
 /* Shallow copy of the input list */

revision.c

@@ -473,20 +473,19 @@ static struct commit *handle_commit(struct rev_info *revs,
 	die("%s is unknown object", name);
 }
 
-static int everybody_uninteresting(struct commit_list *orig,
+static int everybody_uninteresting(struct prio_queue *orig,
 				   struct commit **interesting_cache)
 {
-	struct commit_list *list = orig;
+	size_t i;
 
 	if (*interesting_cache) {
 		struct commit *commit = *interesting_cache;
 		if (!(commit->object.flags & UNINTERESTING))
 			return 0;
 	}
 
-	while (list) {
-		struct commit *commit = list->item;
-		list = list->next;
+	for (i = 0; i < orig->nr; i++) {
+		struct commit *commit = orig->array[i].data;
 		if (commit->object.flags & UNINTERESTING)
 			continue;
 
@@ -1117,7 +1116,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 }
 
 static int process_parents(struct rev_info *revs, struct commit *commit,
-			   struct commit_list **list, struct prio_queue *queue)
+			   struct prio_queue *queue)
 {
 	struct commit_list *parent = commit->parents;
 	unsigned pass_flags;
@@ -1159,8 +1158,6 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
 			if (p->object.flags & SEEN)
 				continue;
 			p->object.flags |= (SEEN | NOT_USER_GIVEN);
-			if (list)
-				commit_list_insert_by_date(p, list);
 			if (queue)
 				prio_queue_put(queue, p);
 			if (revs->exclude_first_parent_only)
@@ -1208,8 +1205,6 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
 		p->object.flags |= pass_flags | CHILD_VISITED;
 		if (!(p->object.flags & SEEN)) {
 			p->object.flags |= (SEEN | NOT_USER_GIVEN);
-			if (list)
-				commit_list_insert_by_date(p, list);
 			if (queue)
 				prio_queue_put(queue, p);
 		}
@@ -1300,20 +1295,17 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 /* How many extra uninteresting commits we want to see.. */
 #define SLOP 5
 
-static int still_interesting(struct commit_list *src, timestamp_t date, int slop,
+static int still_interesting(struct prio_queue *src, timestamp_t date, int slop,
 			     struct commit **interesting_cache)
 {
 	/*
-	 * No source list at all? We're definitely done..
+	 * Since src is sorted by date, it is enough to peek at the
+	 * first entry to compare dates.  No entry at all means done.
 	 */
-	if (!src)
+	struct commit *commit = prio_queue_peek(src);
+	if (!commit)
 		return 0;
-
-	/*
-	 * Does the destination list contain entries with a date
-	 * before the source list? Definitely _not_ done.
-	 */
-	if (date <= src->item->date)
+	if (date <= commit->date)
 		return SLOP;
 
 	/*
@@ -1451,6 +1443,7 @@ static int limit_list(struct rev_info *revs)
 	struct commit_list *newlist = NULL;
 	struct commit_list **p = &newlist;
 	struct commit *interesting_cache = NULL;
+	struct prio_queue queue = { .compare = compare_commits_by_commit_date };
 
 	if (revs->ancestry_path_implicit_bottoms) {
 		collect_bottom_commits(original_list,
@@ -1461,18 +1454,25 @@ static int limit_list(struct rev_info *revs)
 
 	while (original_list) {
 		struct commit *commit = pop_commit(&original_list);
+		prio_queue_put(&queue, commit);
+	}
+
+	while (queue.nr) {
+		struct commit *commit = prio_queue_get(&queue);
 		struct object *obj = &commit->object;
 
 		if (commit == interesting_cache)
 			interesting_cache = NULL;
 
 		if (revs->max_age != -1 && (commit->date < revs->max_age))
 			obj->flags |= UNINTERESTING;
-		if (process_parents(revs, commit, &original_list, NULL) < 0)
+		if (process_parents(revs, commit, &queue) < 0) {
+			clear_prio_queue(&queue);
 			return -1;
+		}
 		if (obj->flags & UNINTERESTING) {
 			mark_parents_uninteresting(revs, commit);
-			slop = still_interesting(original_list, date, slop, &interesting_cache);
+			slop = still_interesting(&queue, date, slop, &interesting_cache);
 			if (slop)
 				continue;
 			break;
@@ -1509,7 +1509,7 @@ static int limit_list(struct rev_info *revs)
 		}
 	}
 
-	commit_list_free(original_list);
+	clear_prio_queue(&queue);
 	revs->commits = newlist;
 	return 0;
 }
@@ -3250,9 +3250,15 @@ static void free_void_commit_list(void *list)
 	commit_list_free(list);
 }
 
+static const struct revision_walk_ops *get_walk_ops(struct rev_info *revs);
+
 void release_revisions(struct rev_info *revs)
 {
+	if (revs->walk_ops && revs->walk_ops != get_walk_ops(revs))
+		BUG("walk_ops changed after initial selection");
+
 	commit_list_free(revs->commits);
+	clear_prio_queue(&revs->commit_queue);
 	commit_list_free(revs->ancestry_path_bottoms);
 	release_display_notes(&revs->notes_opt);
 	object_array_clear(&revs->pending);
@@ -3722,7 +3728,7 @@ static void explore_walk_step(struct rev_info *revs)
 	if (revs->max_age != -1 && (c->date < revs->max_age))
 		c->object.flags |= UNINTERESTING;
 
-	if (process_parents(revs, c, NULL, NULL) < 0)
+	if (process_parents(revs, c, NULL) < 0)
 		return;
 
 	if (c->object.flags & UNINTERESTING)
@@ -3898,7 +3904,7 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
 {
 	struct commit_list *p;
 	struct topo_walk_info *info = revs->topo_walk_info;
-	if (process_parents(revs, commit, NULL, NULL) < 0) {
+	if (process_parents(revs, commit, NULL) < 0) {
 		if (!revs->ignore_missing_links)
 			die("Failed to traverse parents of commit %s",
 			    oid_to_hex(&commit->object.oid));
@@ -3934,12 +3940,34 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
 	}
 }
 
+void rev_info_commit_list_to_queue(struct rev_info *revs)
+{
+	while (revs->commits)
+		prio_queue_put(&revs->commit_queue, pop_commit(&revs->commits));
+}
+
+
+/*
+ * Reset walk machinery so the same rev_info can be walked again.
+ * The bitmap code does this: it walks haves, then walks wants on the
+ * same rev_info. Without this reset, the cached walk_ops would skip
+ * re-initialization (e.g. draining the commit list into the priority
+ * queue), causing the second walk to read from an empty queue.
+ */
+static void reset_walk_state(struct rev_info *revs)
+{
+	revs->walk_ops = NULL;
+	clear_prio_queue(&revs->commit_queue);
+}
+
 int prepare_revision_walk(struct rev_info *revs)
 {
 	int i;
 	struct object_array old_pending;
 	struct commit_list **next = &revs->commits;
 
+	reset_walk_state(revs);
+
 	memcpy(&old_pending, &revs->pending, sizeof(old_pending));
 	revs->pending.nr = 0;
 	revs->pending.alloc = 0;
@@ -4002,7 +4030,7 @@ static enum rewrite_result rewrite_one_1(struct rev_info *revs,
 	for (;;) {
 		struct commit *p = *pp;
 		if (!revs->limited)
-			if (process_parents(revs, p, NULL, queue) < 0)
+			if (process_parents(revs, p, queue) < 0)
 				return rewrite_one_error;
 		if (p->object.flags & UNINTERESTING)
 			return rewrite_one_ok;
@@ -4016,27 +4044,18 @@ static enum rewrite_result rewrite_one_1(struct rev_info *revs,
 	}
 }
 
-static void merge_queue_into_list(struct prio_queue *q, struct commit_list **list)
+static void merge_queue_into_prio_queue(struct prio_queue *from,
+					struct prio_queue *to)
 {
-	while (q->nr) {
-		struct commit *item = prio_queue_peek(q);
-		struct commit_list *p = *list;
-
-		if (p && p->item->date >= item->date)
-			list = &p->next;
-		else {
-			p = commit_list_insert(item, list);
-			list = &p->next; /* skip newly added item */
-			prio_queue_get(q); /* pop item */
-		}
-	}
+	while (from->nr)
+		prio_queue_put(to, prio_queue_get(from));
 }
 
 static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp)
 {
 	struct prio_queue queue = { compare_commits_by_commit_date };
 	enum rewrite_result ret = rewrite_one_1(revs, pp, &queue);
-	merge_queue_into_list(&queue, &revs->commits);
+	merge_queue_into_prio_queue(&queue, &revs->commit_queue);
 	clear_prio_queue(&queue);
 	return ret;
 }
@@ -4323,39 +4342,98 @@ static void track_linear(struct rev_info *revs, struct commit *commit)
 	revs->previous_parents = commit_list_copy(commit->parents);
 }
 
+struct revision_walk_ops {
+	void (*init)(struct rev_info *);
+	struct commit *(*next)(struct rev_info *);
+	int (*expand)(struct rev_info *, struct commit *);
+};
+
+static struct commit *next_reflog(struct rev_info *revs)
+{
+	struct commit *commit = next_reflog_entry(revs->reflog_info);
+	if (commit)
+		commit->object.flags &= ~(ADDED | SEEN | SHOWN);
+	return commit;
+}
+
+static int expand_reflog(struct rev_info *revs, struct commit *commit)
+{
+	try_to_simplify_commit(revs, commit);
+	return 0;
+}
+
+static int expand_topo(struct rev_info *revs, struct commit *commit)
+{
+	expand_topo_walk(revs, commit);
+	return 0;
+}
+
+static struct commit *next_streaming(struct rev_info *revs)
+{
+	return prio_queue_get(&revs->commit_queue);
+}
+
+static int expand_streaming(struct rev_info *revs, struct commit *commit)
+{
+	return process_parents(revs, commit, &revs->commit_queue);
+}
+
+static struct commit *next_commit_list(struct rev_info *revs)
+{
+	return pop_commit(&revs->commits);
+}
+
+static int expand_no_walk(struct rev_info *revs, struct commit *commit)
+{
+	return process_parents(revs, commit, NULL);
+}
+
+static struct revision_walk_ops reflog_ops =
+	{ NULL, next_reflog, expand_reflog };
+static struct revision_walk_ops topo_ops =
+	{ NULL, next_topo_commit, expand_topo };
+static struct revision_walk_ops streaming_ops =
+	{ rev_info_commit_list_to_queue, next_streaming, expand_streaming };
+static struct revision_walk_ops no_walk_ops =
+	{ NULL, next_commit_list, expand_no_walk };
+static struct revision_walk_ops limited_ops =
+	{ NULL, next_commit_list, NULL };
+
+static const struct revision_walk_ops *get_walk_ops(struct rev_info *revs)
+{
+	if (revs->reflog_info)
+		return &reflog_ops;
+	if (revs->topo_walk_info)
+		return &topo_ops;
+	if (revs->no_walk)
+		return &no_walk_ops;
+	if (!revs->limited)
+		return &streaming_ops;
+	return &limited_ops;
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
-	while (1) {
-		struct commit *commit;
+	const struct revision_walk_ops *ops;
 
-		if (revs->reflog_info)
-			commit = next_reflog_entry(revs->reflog_info);
-		else if (revs->topo_walk_info)
-			commit = next_topo_commit(revs);
-		else
-			commit = pop_commit(&revs->commits);
+	if (!revs->walk_ops) {
+		revs->walk_ops = get_walk_ops(revs);
+		if (revs->walk_ops->init)
+			revs->walk_ops->init(revs);
+	}
+	ops = revs->walk_ops;
+
+	while (1) {
+		struct commit *commit = ops->next(revs);
 
 		if (!commit)
 			return NULL;
 
-		if (revs->reflog_info)
-			commit->object.flags &= ~(ADDED | SEEN | SHOWN);
-
-		/*
-		 * If we haven't done the list limiting, we need to look at
-		 * the parents here. We also need to do the date-based limiting
-		 * that we'd otherwise have done in limit_list().
-		 */
-		if (!revs->limited) {
+		if (ops->expand) {
 			if (revs->max_age != -1 &&
 			    comparison_date(revs, commit) < revs->max_age)
 				continue;
-
-			if (revs->reflog_info)
-				try_to_simplify_commit(revs, commit);
-			else if (revs->topo_walk_info)
-				expand_topo_walk(revs, commit);
-			else if (process_parents(revs, commit, &revs->commits, NULL) < 0) {
+			if (ops->expand(revs, commit) < 0) {
 				if (!revs->ignore_missing_links)
 					die("Failed to traverse parents of commit %s",
 						oid_to_hex(&commit->object.oid));