Skip to contentSkip to conversation

walker: avoid quadratic list insertion in mark_complete

PATCH In review René Scharfe 2 participants 2 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 walker: avoid quadratic list insertion in mark_complete

Similar to 16445242 (fetch-pack: avoid quadratic list insertion in mark_complete), sort only after all refs are collected instead of while inserting. The result is the same, but it's more efficient that way. The difference will only be measurable in repositories with a large number of refs.

---
 walker.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
walker.c +4 -2
index 0148264..0596e99 100644
@@ -205,7 +205,7 @@ static int mark_complete(const char *path, const unsigned char *sha1, int flag,
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	if (commit) {
 		commit->object.flags |= COMPLETE;
-		commit_list_insert_by_date(commit, &complete);
+		commit_list_insert(commit, &complete);
 	}
 	return 0;
 }
@@ -271,8 +271,10 @@ int walker_fetch(struct walker *walker, int targets, char **target,
 		}
 	}
 
-	if (!walker->get_recover)
+	if (!walker->get_recover) {
 		for_each_ref(mark_complete, NULL);
+		commit_list_sort_by_date(&complete);
+	}
 
 	for (i = 0; i < targets; i++) {
 		if (interpret_target(walker, target[i], &sha1[20 * i])) {
Signature · 1 line
2.1.0
#1 lore raw
Quoted 5 lines · René Scharfe

Similar to 16445242 (fetch-pack: avoid quadratic list insertion in mark_complete), sort only after all refs are collected instead of while inserting. The result is the same, but it's more efficient that way. The difference will only be measurable in repositories with a large number of refs.

Thanks, this looks obviously correct.

I wonder if we should do this on top:

walker.c +1 -1
index 0148264..70088b8 100644
@@ -203,7 +203,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
 static int mark_complete(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
-	if (commit) {
+	if (commit && !(commit->object.flags & COMPLETE)) {
 		commit->object.flags |= COMPLETE;
 		commit_list_insert_by_date(commit, &complete);
 	}

It's not as big a deal with your patch since you've made it O(n log n), but reducing n further does not hurt.

-Peff