Re: [PATCH] commit-reach: simplify cleanup of remaining bitmaps in ahead_behind()
- From
René Scharfe <l.s.r@web.de>
- Date
- Mar 18, 2026, 16:09 UTC
- Message-ID
- <c01eb1e3-d839-4cf6-ba47-5a9edd336ae3@web.de>
- In-Reply-To
- <06000e28-c1b1-472f-bd6b-367b6c8d208d@web.de>
On 3/18/26 1:45 PM, René Scharfe wrote:
Show 5 quoted lines
> Use the deep clear function of the bit_arrays commit slab to free > bitmaps of commits we didn't traverse. We don't care about their order > anymore at this point, so we can bypass the prio_queue and its heap > rebalancing logic. Note that bitmap_free() handles NULL pointers, so we > don't have to check.
That's nice and all, but it's also slower:
Benchmark 1: ./git_main for-each-ref --format='%(objectname) %(ahead-behind:main)' Time (mean ± σ): 1.228 s ± 0.001 s [User: 1.188 s, System: 0.039 s] Range (min … max): 1.226 s … 1.231 s 10 runs
Benchmark 2: ./git_deep_clear for-each-ref --format='%(objectname) %(ahead-behind:main)' Time (mean ± σ): 1.354 s ± 0.002 s [User: 1.313 s, System: 0.039 s] Range (min … max): 1.351 s … 1.356 s 10 runs
Summary
./git_main for-each-ref --format='%(objectname) %(ahead-behind:main)' ran
1.10 ± 0.00 times faster than ./git_deep_clear for-each-ref --format='%(objectname) %(ahead-behind:main)'Please don't apply this patch -- I should have measured first.
Show 31 quoted lines
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> commit-reach.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/commit-reach.c b/commit-reach.c
> index 9604bbdcce..a4fc41ff40 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -1047,6 +1047,11 @@ static void free_bit_array(struct commit *c)
> *bitmap = NULL;
> }
>
> +static void free_bitmap_pointer(struct bitmap **bitmap)
> +{
> + bitmap_free(*bitmap);
> +}
> +
> void ahead_behind(struct repository *r,
> struct commit **commits, size_t commits_nr,
> struct ahead_behind_count *counts, size_t counts_nr)
> @@ -1117,11 +1122,7 @@ void ahead_behind(struct repository *r,
>
> /* STALE is used here, PARENT2 is used by insert_no_dup(). */
> repo_clear_commit_marks(r, PARENT2 | STALE);
> - while (prio_queue_peek(&queue)) {
> - struct commit *c = prio_queue_get(&queue);
> - free_bit_array(c);
> - }
> - clear_bit_arrays(&bit_arrays);
> + deep_clear_bit_arrays(&bit_arrays, free_bitmap_pointer);The prio_queue contains just a few unvisited entries at this point (or perhaps even none), while deep_clear_*() will visit all commits that ever had a bitmap, even if their bitmap pointer is NULL now.
We could still access them in array order, which must be cheaper:
for (size_t i = 0; i < queue.nr; i++) free_bit_array(queue.array[i].data);
Performance is the same for my local Git repo clone, though.
> clear_prio_queue(&queue); > } >