git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Derrick Stolee <dstolee@microsoft.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/4] mark_parents_uninteresting(): replace list with stack
Date: Fri, 11 May 2018 14:02:27 -0400	[thread overview]
Message-ID: <20180511180227.GC12543@sigill.intra.peff.net> (raw)
In-Reply-To: <20180511180029.GA11290@sigill.intra.peff.net>

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>
---
The diff makes a lot more sense with "-w".

 revision.c | 67 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 24 deletions(-)

diff --git a/revision.c b/revision.c
index cee4f3a4b4..89ff9a99ce 100644
--- a/revision.c
+++ b/revision.c
@@ -92,38 +92,57 @@ void mark_tree_uninteresting(struct tree *tree)
 	mark_tree_contents_uninteresting(tree);
 }
 
-void mark_parents_uninteresting(struct commit *commit)
+struct commit_stack {
+	struct commit **items;
+	size_t nr, alloc;
+};
+#define COMMIT_STACK_INIT { NULL, 0, 0 }
+
+static void commit_stack_push(struct commit_stack *stack, struct commit *commit)
 {
-	struct commit_list *parents = NULL, *l;
+	ALLOC_GROW(stack->items, stack->nr + 1, stack->alloc);
+	stack->items[stack->nr++] = commit;
+}
 
-	for (l = commit->parents; l; l = l->next)
-		commit_list_insert(l->item, &parents);
+static struct commit *commit_stack_pop(struct commit_stack *stack)
+{
+	return stack->nr ? stack->items[--stack->nr] : NULL;
+}
 
-	while (parents) {
-		struct commit *commit = pop_commit(&parents);
+static void commit_stack_clear(struct commit_stack *stack)
+{
+	FREE_AND_NULL(stack->items);
+	stack->nr = stack->alloc = 0;
+}
 
-		while (commit) {
-			if (commit->object.flags & UNINTERESTING)
-				break;
+void mark_parents_uninteresting(struct commit *commit)
+{
+	struct commit_stack pending = COMMIT_STACK_INIT;
+	struct commit_list *l;
 
-			commit->object.flags |= UNINTERESTING;
+	for (l = commit->parents; l; l = l->next)
+		commit_stack_push(&pending, l->item);
 
-			/*
-			 * Normally we haven't parsed the parent
-			 * yet, so we won't have a parent of a parent
-			 * here. However, it may turn out that we've
-			 * reached this commit some other way (where it
-			 * wasn't uninteresting), in which case we need
-			 * to mark its parents recursively too..
-			 */
-			if (!commit->parents)
-				break;
+	while (pending.nr > 0) {
+		struct commit *commit = commit_stack_pop(&pending);
 
-			for (l = commit->parents->next; l; l = l->next)
-				commit_list_insert(l->item, &parents);
-			commit = commit->parents->item;
-		}
+		if (commit->object.flags & UNINTERESTING)
+			return;
+		commit->object.flags |= UNINTERESTING;
+
+		/*
+		 * Normally we haven't parsed the parent
+		 * yet, so we won't have a parent of a parent
+		 * here. However, it may turn out that we've
+		 * reached this commit some other way (where it
+		 * wasn't uninteresting), in which case we need
+		 * to mark its parents recursively too..
+		 */
+		for (l = commit->parents; l; l = l->next)
+			commit_stack_push(&pending, l->item);
 	}
+
+	commit_stack_clear(&pending);
 }
 
 static void add_pending_object_with_path(struct rev_info *revs,
-- 
2.17.0.988.gec4b43b3e5


  parent reply	other threads:[~2018-05-11 18:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-11 18:00 [PATCH 0/4] a few mark_parents_uninteresting cleanups Jeff King
2018-05-11 18:00 ` [PATCH 1/4] mark_tree_contents_uninteresting(): drop missing object check Jeff King
2018-05-11 18:01 ` [PATCH 2/4] mark_parents_uninteresting(): " Jeff King
2018-05-13  2:23   ` Junio C Hamano
2018-05-11 18:02 ` Jeff King [this message]
2018-05-11 18:03 ` [PATCH 4/4] mark_parents_uninteresting(): avoid most allocation Jeff King
2018-05-14 12:47   ` Derrick Stolee
2018-05-14 13:09     ` Jeff King
2018-05-14 13:25       ` Derrick Stolee
2018-05-14 14:09         ` Jeff King
2018-05-14 12:50 ` [PATCH 0/4] a few mark_parents_uninteresting cleanups Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180511180227.GC12543@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).