git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] describe: use oidset in finish_depth_computation()

From
René Scharfe <l.s.r@web.de>
Date
Sep 2, 2025, 18:24 UTC
Message-ID
<b16aa099-b683-4a66-acd9-603cc9f0935c@web.de>
In-Reply-To
<9110f085-aec0-42e9-9774-b153ece6284f@web.de>

Depth computation can end early if all remaining commits are flagged. The current code determines if that's the case by checking all queue items each time it dequeues a flagged commit. This can cause quadratic complexity.

We could simply count the flagged items in the queue and then update that number as we add and remove items. That would provide a general speedup, but leave one case where we have to scan the whole queue: When we flag a previously seen, but unflagged commit. It could be on the queue and then we'd have to decrease our count.

We could dedicate an object flag to track queue membership, but that would leave less for candidate tags, affecting the results. So use a hash table, specifically an oidset of commit hashes, to track that. This avoids quadratic behaviour in all cases and provides a nice performance boost over the previous commit, 08bb69d70f (describe: use prio_queue_replace(), 2025-08-03):

Benchmark 1: ./git_08bb69d70f describe $(git rev-list v2.41.0..v2.47.0)
  Time (mean ± σ):     855.3 ms ±   1.3 ms    [User: 790.8 ms, System: 49.9 ms]
  Range (min … max):   853.7 ms … 857.8 ms    10 runs
Benchmark 2: ./git describe $(git rev-list v2.41.0..v2.47.0)
  Time (mean ± σ):     610.8 ms ±   1.7 ms    [User: 546.9 ms, System: 49.3 ms]
  Range (min … max):   608.9 ms … 613.3 ms    10 runs
Summary
  ./git describe $(git rev-list v2.41.0..v2.47.0) ran
    1.40 ± 0.00 times faster than ./git_08bb69d70f describe $(git rev-list v2.41.0..v2.47.0)
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Changes:
- Use oidset instead of a custom khash set for simplicity.
- Removed spurious whitespace changes.
- Formatted with --patience to better see the function removal.
 builtin/describe.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index fbe78ace66..9f4e26d7ff 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -24,6 +24,7 @@
 #include "commit-slab.h"
 #include "wildmatch.h"
 #include "prio-queue.h"
+#include "oidset.h"
 
 #define MAX_TAGS	(FLAG_BITS - 1)
 #define DEFAULT_CANDIDATES 10
@@ -286,38 +287,47 @@ static void lazy_queue_clear(struct lazy_queue *queue)
 	queue->get_pending = false;
 }
 
-static bool all_have_flag(const struct lazy_queue *queue, unsigned flag)
-{
-	for (size_t i = queue->get_pending ? 1 : 0; i < queue->queue.nr; i++) {
-		struct commit *commit = queue->queue.array[i].data;
-		if (!(commit->object.flags & flag))
-			return false;
-	}
-	return true;
-}
-
 static unsigned long finish_depth_computation(struct lazy_queue *queue,
 					      struct possible_tag *best)
 {
 	unsigned long seen_commits = 0;
+	struct oidset unflagged = OIDSET_INIT;
+
+	for (size_t i = queue->get_pending ? 1 : 0; i < queue->queue.nr; i++) {
+		struct commit *commit = queue->queue.array[i].data;
+		if (!(commit->object.flags & best->flag_within))
+			oidset_insert(&unflagged, &commit->object.oid);
+	}
+
 	while (!lazy_queue_empty(queue)) {
 		struct commit *c = lazy_queue_get(queue);
 		struct commit_list *parents = c->parents;
 		seen_commits++;
 		if (c->object.flags & best->flag_within) {
-			if (all_have_flag(queue, best->flag_within))
+			if (!oidset_size(&unflagged))
 				break;
-		} else
+		} else {
+			oidset_remove(&unflagged, &c->object.oid);
 			best->depth++;
+		}
 		while (parents) {
+			unsigned seen, flag_before, flag_after;
 			struct commit *p = parents->item;
 			repo_parse_commit(the_repository, p);
-			if (!(p->object.flags & SEEN))
+			seen = p->object.flags & SEEN;
+			if (!seen)
 				lazy_queue_put(queue, p);
+			flag_before = p->object.flags & best->flag_within;
 			p->object.flags |= c->object.flags;
+			flag_after = p->object.flags & best->flag_within;
+			if (!seen && !flag_after)
+				oidset_insert(&unflagged, &p->object.oid);
+			if (seen && !flag_before && flag_after)
+				oidset_remove(&unflagged, &p->object.oid);
 			parents = parents->next;
 		}
 	}
+	oidset_clear(&unflagged);
 	return seen_commits;
 }
 
-- 
2.51.0
Previous: Jeff KingNext: Jeff King
Message 20 of 21 in “describe: use khash in finish_depth_computation()”
  1. describe: use khash in finish_depth_computation()René Scharfe, Aug 24, 2025
  2. Jeff KingAug 24, 2025
  3. René ScharfeAug 24, 2025
  4. Jeff KingAug 25, 2025
  5. Jeff KingAug 25, 2025
  6. Junio C HamanoAug 25, 2025
  7. Jeff KingAug 26, 2025
  8. Jeff KingAug 26, 2025
  9. Jeff KingAug 26, 2025
  10. Junio C HamanoAug 26, 2025
  11. René ScharfeAug 31, 2025
  12. René ScharfeSep 1, 2025
  13. Jeff KingSep 2, 2025
  14. René ScharfeSep 2, 2025
  15. Jeff KingSep 3, 2025
  16. René ScharfeSep 3, 2025
  17. Jeff KingSep 4, 2025
  18. René ScharfeSep 3, 2025
  19. Jeff KingSep 4, 2025
  20. describe: use oidset in finish_depth_computation()René Scharfe, Sep 2, 2025
  21. Jeff KingSep 3, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.