Re: [PATCH 09/13] rev-list: use bitmap filters for traversal

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 13, 2020, 22:22 UTC
Message-ID: <xmqqblq2qga8.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20200213022125.GI1126038@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> This just passes the filter-options struct to prepare_bitmap_walk().
> Since the bitmap code doesn't actually support any filters yet, it will
> fallback to the non-bitmap code if any --filter is specified. But this
> lets us exercise that rejection code path, as well as getting us ready
> to test filters via rev-list when we _do_ support them.

So we used to look at filter_options.choice and declared any filter is incompatible with use_bitmap_index quite early, but now we let each of the try_bitmap_*() helpers check what is in the filter and make their own decisions.

Of course, the prepare_bitmap_walk() call at the beginning of these helpers does not know how to work with any filter at this point in the series, so all of the above cancel out :-).

Makes sense.

I wonder if the "revs.prune" thing that forces use_bitmap_index off should also move to prepare_bitmap_walk() at some point in the series (or after the current series is done). After all, the point of introducing try_bitmap_*() helpers was to let these bitmap specific logic to know what is and is not compatible with the bitmap routines.

Thanks.

Show 29 quoted lines

> @@ -441,7 +443,7 @@ static int try_bitmap_traversal(struct rev_info *revs)
>  	if (!revs->tag_objects || !revs->tree_objects || !revs->blob_objects)
>  		return -1;
>  
> -	bitmap_git = prepare_bitmap_walk(revs, NULL);
> +	bitmap_git = prepare_bitmap_walk(revs, filter);
>  	if (!bitmap_git)
>  		return -1;
>  
> @@ -612,7 +614,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  	    (revs.left_right || revs.cherry_mark))
>  		die(_("marked counting is incompatible with --objects"));
>  
> -	if (filter_options.choice || revs.prune)
> +	if (revs.prune)
>  		use_bitmap_index = 0;
>  
>  	save_commit_buffer = (revs.verbose_header ||
> @@ -625,9 +627,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  		progress = start_delayed_progress(show_progress, 0);
>  
>  	if (use_bitmap_index) {
> -		if (!try_bitmap_count(&revs))
> +		if (!try_bitmap_count(&revs, &filter_options))
>  			return 0;
> -		if (!try_bitmap_traversal(&revs))
> +		if (!try_bitmap_traversal(&revs, &filter_options))
>  			return 0;
>  	}