{"thread":{"id":"58941","subject":"[PATCH] commit: skip already cleared parents in clear_commit_marks_1()","startedAt":"2022-12-13T06:27:25Z","lastAt":"2022-12-15T15:14:47Z","messageCount":2,"participants":["René Scharfe","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"468971","messageId":"7cf2ea1f-8dbf-5639-3874-86de391ae20a@web.de","threadId":"58941","inReplyTo":null,"subject":"[PATCH] commit: skip already cleared parents in clear_commit_marks_1()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2022-12-13T06:27:10Z","receivedAt":"2022-12-13T06:27:25Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Don't put clean parents on the pending list, as they and their ancestors\ndon't need any treatment and would be skipped later anyway.  This saves\nthe allocation and release of a commit list item in ca. 20% of the cases\nduring a run of the test suite.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n commit.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 572301b80a..d00780bee5 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -701,8 +701,10 @@ static void clear_commit_marks_1(struct commit_list **plist,\n \t\tif (!parents)\n \t\t\treturn;\n\n-\t\twhile ((parents = parents->next))\n-\t\t\tcommit_list_insert(parents->item, plist);\n+\t\twhile ((parents = parents->next)) {\n+\t\t\tif (parents->item->object.flags & mark)\n+\t\t\t\tcommit_list_insert(parents->item, plist);\n+\t\t}\n\n \t\tcommit = commit->parents->item;\n \t}\n--\n2.38.2\n"},{"id":"469129","messageId":"112d189c-f2f9-8e12-fe13-82e41316c41b@github.com","threadId":"58941","inReplyTo":"7cf2ea1f-8dbf-5639-3874-86de391ae20a@web.de","subject":"Re: [PATCH] commit: skip already cleared parents in clear_commit_marks_1()","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-12-15T15:13:59Z","receivedAt":"2022-12-15T15:14:47Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 12/13/22 1:27 AM, René Scharfe wrote:\n> Don't put clean parents on the pending list, as they and their ancestors\n> don't need any treatment and would be skipped later anyway.  This saves\n> the allocation and release of a commit list item in ca. 20% of the cases\n> during a run of the test suite.\n\n> -\t\twhile ((parents = parents->next))\n> -\t\t\tcommit_list_insert(parents->item, plist);\n> +\t\twhile ((parents = parents->next)) {\n> +\t\t\tif (parents->item->object.flags & mark)\n> +\t\t\t\tcommit_list_insert(parents->item, plist);\n> +\t\t}\n\nFrom this context, it looks like this is a change of behavior (the\nwalk only continues to parents that have the flag, so if the flag\nis not closed under reachability, such as the UNINTERESTING flag,\nthen we won't clear all flags) but above this we already skip a\ncommit that doesn't have the flag enabled.\n\nOne thing that is odd about this method is that it sets the\n'commit' value to the first parent, and adds the later parents\nto the list. It's a minor point, but that check for the flag\non the 'commit' is what prevents us from walking too far along\nthe first-parent history.\n\nAll this to say, this is a safe change with a nice upside.\n\nThanks,\n-Stolee\n"}]}