diff mbox series

[v4,3/4] pack-bitmap.c: break out of the bitmap loop early if not tracing

Message ID f6c5b45bdcfd05eba2984163edf38d9915255047.1669032426.git.dyroneteng@gmail.com (mailing list archive)
State Superseded
Headers show
Series pack-bitmap.c: avoid exposing absolute paths | expand

Commit Message

Teng Long Nov. 21, 2022, 12:16 p.m. UTC
From: Jeff King <peff@peff.net>

When we successfully open a bitmap, we will continue to try to open
other packs, and when trace2 is enabled, we will report any subsequent
bitmap ignored information in the log. So when we find that trace2 is
not enabled, we can actually terminate the loop early.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 pack-bitmap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Junio C Hamano Nov. 21, 2022, 11:27 p.m. UTC | #1
Teng Long <dyroneteng@gmail.com> writes:

> From: Jeff King <peff@peff.net>
>
> When we successfully open a bitmap, we will continue to try to open
> other packs, and when trace2 is enabled, we will report any subsequent
> bitmap ignored information in the log. So when we find that trace2 is
> not enabled, we can actually terminate the loop early.

The above took me a few reads to understand what it wants to say,
probably because the "and when trace2 is enabled" comes a bit too
late to explain why "try to open other" is done.  After reading it a
few times, here is what I think it wants to say:

    After opening a bitmap successfully, we try opening others only
    because we want to report that other bitmap files are ignored in
    the trace2 log.  When trace2 is not enabled, we do not have to
    do any of that.



>
> Signed-off-by: Jeff King <peff@peff.net>
> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
>  pack-bitmap.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index aaa2d9a104..3b6c2f804a 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -527,8 +527,15 @@ static int open_pack_bitmap(struct repository *r,
>  	assert(!bitmap_git->map);
>  
>  	for (p = get_all_packs(r); p; p = p->next) {
> -		if (open_pack_bitmap_1(bitmap_git, p) == 0)
> +		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
>  			ret = 0;
> +			/*
> +			 * The only reason to keep looking is to report
> +			 * duplicates.
> +			 */
> +			if (!trace2_is_enabled())
> +				break;
> +		}
>  	}
>  
>  	return ret;
Teng Long Nov. 28, 2022, 1:09 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

> From: Jeff King <peff@peff.net>
>
> When we successfully open a bitmap, we will continue to try to open
> other packs, and when trace2 is enabled, we will report any subsequent
> bitmap ignored information in the log. So when we find that trace2 is
> not enabled, we can actually terminate the loop early.

> The above took me a few reads to understand what it wants to say,
> probably because the "and when trace2 is enabled" comes a bit too
> late to explain why "try to open other" is done.  After reading it a
> few times, here is what I think it wants to say:
>
>     After opening a bitmap successfully, we try opening others only
>     because we want to report that other bitmap files are ignored in
>     the trace2 log.  When trace2 is not enabled, we do not have to
>     do any of that.

Thanks for the reword, will apply in reroll.
diff mbox series

Patch

diff --git a/pack-bitmap.c b/pack-bitmap.c
index aaa2d9a104..3b6c2f804a 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -527,8 +527,15 @@  static int open_pack_bitmap(struct repository *r,
 	assert(!bitmap_git->map);
 
 	for (p = get_all_packs(r); p; p = p->next) {
-		if (open_pack_bitmap_1(bitmap_git, p) == 0)
+		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
 			ret = 0;
+			/*
+			 * The only reason to keep looking is to report
+			 * duplicates.
+			 */
+			if (!trace2_is_enabled())
+				break;
+		}
 	}
 
 	return ret;