diff mbox series

commit: skip already cleared parents in clear_commit_marks_1()

Message ID 7cf2ea1f-8dbf-5639-3874-86de391ae20a@web.de (mailing list archive)
State New, archived
Headers show
Series commit: skip already cleared parents in clear_commit_marks_1() | expand

Commit Message

René Scharfe Dec. 13, 2022, 6:27 a.m. UTC
Don't put clean parents on the pending list, as they and their ancestors
don't need any treatment and would be skipped later anyway.  This saves
the allocation and release of a commit list item in ca. 20% of the cases
during a run of the test suite.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 commit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
2.38.2

Comments

Derrick Stolee Dec. 15, 2022, 3:13 p.m. UTC | #1
On 12/13/22 1:27 AM, René Scharfe wrote:
> Don't put clean parents on the pending list, as they and their ancestors
> don't need any treatment and would be skipped later anyway.  This saves
> the allocation and release of a commit list item in ca. 20% of the cases
> during a run of the test suite.

> -		while ((parents = parents->next))
> -			commit_list_insert(parents->item, plist);
> +		while ((parents = parents->next)) {
> +			if (parents->item->object.flags & mark)
> +				commit_list_insert(parents->item, plist);
> +		}

From this context, it looks like this is a change of behavior (the
walk only continues to parents that have the flag, so if the flag
is not closed under reachability, such as the UNINTERESTING flag,
then we won't clear all flags) but above this we already skip a
commit that doesn't have the flag enabled.

One thing that is odd about this method is that it sets the
'commit' value to the first parent, and adds the later parents
to the list. It's a minor point, but that check for the flag
on the 'commit' is what prevents us from walking too far along
the first-parent history.

All this to say, this is a safe change with a nice upside.

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/commit.c b/commit.c
index 572301b80a..d00780bee5 100644
--- a/commit.c
+++ b/commit.c
@@ -701,8 +701,10 @@  static void clear_commit_marks_1(struct commit_list **plist,
 		if (!parents)
 			return;

-		while ((parents = parents->next))
-			commit_list_insert(parents->item, plist);
+		while ((parents = parents->next)) {
+			if (parents->item->object.flags & mark)
+				commit_list_insert(parents->item, plist);
+		}

 		commit = commit->parents->item;
 	}