mark_parents_uninteresting(): replace list with stack

The mark_parents_uninteresting() function uses two loops: an outer one to process our queue of pending parents, and an inner one to process first-parent chains. This is a clever optimization from 941ba8db57 (Eliminate recursion in setting/clearing marks in commit list, 2012-01-14) to limit the number of linked-list allocations when following single-parent chains. Unfortunately, this makes the result a little hard to read. Let's replace the list with a stack. Then we don't have to worry about doing this double-loop optimization, as we'll just reuse the top element of the stack as we pop/push. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 11, 2018 at 14:02 UTC 43fc643b75af91363eb1246528c706c1654ddc2e
1 file changed +43 -24
revision.c
+43 -24
@@ -91,38 +91,57 @@ void mark_tree_uninteresting(struct tree *tree)
91 mark_tree_contents_uninteresting(tree);
92 }
93
94 -void mark_parents_uninteresting(struct commit *commit)
94 +struct commit_stack {
95 + struct commit **items;
96 + size_t nr, alloc;
97 +};
98 +#define COMMIT_STACK_INIT { NULL, 0, 0 }
99 +
100 +static void commit_stack_push(struct commit_stack *stack, struct commit *commit)
101 {
96 - struct commit_list *parents = NULL, *l;
102 + ALLOC_GROW(stack->items, stack->nr + 1, stack->alloc);
103 + stack->items[stack->nr++] = commit;
104 +}
105
98 - for (l = commit->parents; l; l = l->next)
99 - commit_list_insert(l->item, &parents);
106 +static struct commit *commit_stack_pop(struct commit_stack *stack)
107 +{
108 + return stack->nr ? stack->items[--stack->nr] : NULL;
109 +}
110
101 - while (parents) {
102 - struct commit *commit = pop_commit(&parents);
111 +static void commit_stack_clear(struct commit_stack *stack)
112 +{
113 + FREE_AND_NULL(stack->items);
114 + stack->nr = stack->alloc = 0;
115 +}
116
104 - while (commit) {
105 - if (commit->object.flags & UNINTERESTING)
106 - break;
117 +void mark_parents_uninteresting(struct commit *commit)
118 +{
119 + struct commit_stack pending = COMMIT_STACK_INIT;
120 + struct commit_list *l;
121
108 - commit->object.flags |= UNINTERESTING;
122 + for (l = commit->parents; l; l = l->next)
123 + commit_stack_push(&pending, l->item);
124
110 - /*
111 - * Normally we haven't parsed the parent
112 - * yet, so we won't have a parent of a parent
113 - * here. However, it may turn out that we've
114 - * reached this commit some other way (where it
115 - * wasn't uninteresting), in which case we need
116 - * to mark its parents recursively too..
117 - */
118 - if (!commit->parents)
119 - break;
125 + while (pending.nr > 0) {
126 + struct commit *commit = commit_stack_pop(&pending);
127
121 - for (l = commit->parents->next; l; l = l->next)
122 - commit_list_insert(l->item, &parents);
123 - commit = commit->parents->item;
124 - }
128 + if (commit->object.flags & UNINTERESTING)
129 + return;
130 + commit->object.flags |= UNINTERESTING;
131 +
132 + /*
133 + * Normally we haven't parsed the parent
134 + * yet, so we won't have a parent of a parent
135 + * here. However, it may turn out that we've
136 + * reached this commit some other way (where it
137 + * wasn't uninteresting), in which case we need
138 + * to mark its parents recursively too..
139 + */
140 + for (l = commit->parents; l; l = l->next)
141 + commit_stack_push(&pending, l->item);
142 }
143 +
144 + commit_stack_clear(&pending);
145 }
146
147 static void add_pending_object_with_path(struct rev_info *revs,