Eliminate recursion in setting/clearing marks in commit list

Recursion in a DAG is generally a bad idea because it could be very deep. Be defensive and avoid recursion in mark_parents_uninteresting() and clear_commit_marks(). mark_parents_uninteresting() learns a trick from clear_commit_marks() to avoid malloc() in (dominant) single-parent case. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jan 14, 2012 at 19:19 UTC 941ba8db57f2d075aee48b002ee30686288cb502
2 files changed +40 -18
commit.c
+11 -2
@@ -421,7 +421,8 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
421 return ret;
422 }
423
424 -void clear_commit_marks(struct commit *commit, unsigned int mark)
424 +static void clear_commit_marks_1(struct commit_list **plist,
425 + struct commit *commit, unsigned int mark)
426 {
427 while (commit) {
428 struct commit_list *parents;
@@ -436,12 +437,20 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
437 return;
438
439 while ((parents = parents->next))
439 - clear_commit_marks(parents->item, mark);
440 + commit_list_insert(parents->item, plist);
441
442 commit = commit->parents->item;
443 }
444 }
445
446 +void clear_commit_marks(struct commit *commit, unsigned int mark)
447 +{
448 + struct commit_list *list = NULL;
449 + commit_list_insert(commit, &list);
450 + while (list)
451 + clear_commit_marks_1(&list, pop_commit(&list), mark);
452 +}
453 +
454 void clear_commit_marks_for_object_array(struct object_array *a, unsigned mark)
455 {
456 struct object *object;
revision.c
+29 -16
@@ -139,11 +139,32 @@ void mark_tree_uninteresting(struct tree *tree)
139
140 void mark_parents_uninteresting(struct commit *commit)
141 {
142 - struct commit_list *parents = commit->parents;
142 + struct commit_list *parents = NULL, *l;
143 +
144 + for (l = commit->parents; l; l = l->next)
145 + commit_list_insert(l->item, &parents);
146
147 while (parents) {
148 struct commit *commit = parents->item;
146 - if (!(commit->object.flags & UNINTERESTING)) {
149 + l = parents;
150 + parents = parents->next;
151 + free(l);
152 +
153 + while (commit) {
154 + /*
155 + * A missing commit is ok iff its parent is marked
156 + * uninteresting.
157 + *
158 + * We just mark such a thing parsed, so that when
159 + * it is popped next time around, we won't be trying
160 + * to parse it and get an error.
161 + */
162 + if (!has_sha1_file(commit->object.sha1))
163 + commit->object.parsed = 1;
164 +
165 + if (commit->object.flags & UNINTERESTING)
166 + break;
167 +
168 commit->object.flags |= UNINTERESTING;
169
170 /*
@@ -154,21 +175,13 @@ void mark_parents_uninteresting(struct commit *commit)
175 * wasn't uninteresting), in which case we need
176 * to mark its parents recursively too..
177 */
157 - if (commit->parents)
158 - mark_parents_uninteresting(commit);
159 - }
178 + if (!commit->parents)
179 + break;
180
161 - /*
162 - * A missing commit is ok iff its parent is marked
163 - * uninteresting.
164 - *
165 - * We just mark such a thing parsed, so that when
166 - * it is popped next time around, we won't be trying
167 - * to parse it and get an error.
168 - */
169 - if (!has_sha1_file(commit->object.sha1))
170 - commit->object.parsed = 1;
171 - parents = parents->next;
181 + for (l = commit->parents->next; l; l = l->next)
182 + commit_list_insert(l->item, &parents);
183 + commit = commit->parents->item;
184 + }
185 }
186 }
187