Re: [PATCH v2] show-branch: convert object.flags to commit-slab with uint64_t

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 14, 2026, 20:41 UTC
Message-ID: <xmqqcxwps3ro.fsf@gitster.g>
In-Reply-To: <20260714200237.70509-1-gatlavishweshwarreddy26@gmail.com>

Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:

Show 5 quoted lines

> show-branch uses commit->object.flags to store per-commit data:
> the UNINTERESTING bit and per-branch reachability bits. Using the
> shared object.flags field for this purpose is fragile as it
> conflicts with other users of the same field, and limits the
> number of branches that can be shown to MAX_REVS (27).

The command was written with the understanding that it would not allow other parts of the system to touch these per-object flag bits. Therefore, fragility is not a relevant issue. The primary problem with this design is that the flags word has only a fixed number of available bits, meaning it cannot process hundreds of branches simultaneously.

This limitation is precisely where the concept of using a commit slab shines. However, to truly take advantage of a commit slab, the slab stride must be variable. If the tool is handling more than 80 branches, for example, each commit requires a `uint64_t[2]` array allocation (since a single `uint64_t` provides only 64 bits, while `uint64_t[2]` can store up to 128 bits).

Show 5 quoted lines

> Convert this usage to a dedicated commit-slab using uint64_t as
> the element type. This is the canonical way to associate per-commit
> data in Git without polluting the shared object flags. Using
> uint64_t instead of unsigned int lifts the MAX_REVS limitation
> from 27 to 62 branches, as suggested in prior review discussions.

I do not understand the reference to 62. As I previously noted, storing a fixed uint64_t[1] instead of variable-length uint64_t[n] in each slab entry fails to realize the full potential of using commit slabs. Furthermore, we should be able to utilize all 64 bits of a uint64_t word. There is no need to pollute this dedicated, one-bit-per-branch slab with the UNINTERESTING bit, which is used for the command's revision walking. Revision walking can continue using the UNINTERESTING bit in the standard object.flags instead.

Show 24 quoted lines

> @@ -511,18 +523,20 @@ static int rev_is_head(const char *head, const char *name)
>
>  static int show_merge_base(const struct commit_list *seen, int num_rev)
>  {
> -	int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
> -	int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
> +	uint64_t all_mask = ((UINT64_C(1) << (REV_SHIFT + num_rev)) - 1);
> +	uint64_t all_revs = all_mask & ~((UINT64_C(1) << REV_SHIFT) - 1);
>  	int exit_status = 1;
>
>  	for (const struct commit_list *s = seen; s; s = s->next) {
>  		struct commit *commit = s->item;
> -		int flags = commit->object.flags & all_mask;
> +		uint64_t flags = get_rev_flags(commit) & all_mask;
>  		if (!(flags & UNINTERESTING) &&
>  		    ((flags & all_revs) == all_revs)) {
>  			puts(oid_to_hex(&commit->object.oid));
>  			exit_status = 0;
> -			commit->object.flags |= UNINTERESTING;
> +
> +or_rev_flags(commit, UNINTERESTING);
> +
>  		}
>  	}

What's this funny indentation?

Show 6 quoted lines

> @@ -607,9 +621,9 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
>  	for (i = 0; i < n; i++)
>  		if (rev[i] == commit)
>  			return 0;
> -	flag = commit->object.flags;
> +	flag = get_rev_flags(commit);

Has the definition of local variable "flag" in omit_in_dense() been updated to u64? If it is still "int", then this would not work well on platforms whose "int" is still i32.

Show 6 quoted lines

>  	for (i = count = 0; i < n; i++) {
> -		if (flag & (1u << (i + REV_SHIFT)))
> +		if (flag & (UINT64_C(1) << (i + REV_SHIFT)))
>  			count++;
>  	}
>  	if (count == 1)