[PATCH] show-branch: convert object.flags usage to a commit-slab

[PATCH] show-branch: convert object.flags usage to a commit-slab

[Gatla Vishweshwar Reddy]

show-branch uses commit->object.flags to store two kinds of
per-commit data: the UNINTERESTING bit to mark commits that are
ancestors of all given revisions, and per-branch reachability
bits (one bit per branch, starting at REV_SHIFT) to track which
branches can reach each commit.

Using the shared object.flags field for this purpose is fragile.
The field is shared across the entire Git codebase and other
subsystems use it for their own bookkeeping. Storing show-branch
specific data there risks conflicts with other users of the same
field.

Convert this usage to a dedicated commit-slab named
commit_rev_flags, which is the canonical way to associate
per-commit data in Git without polluting the shared object flags.
Add helper functions get_rev_flags() and or_rev_flags() to
encapsulate slab access cleanly, and initialize and clear the
slab in cmd_show_branch() to avoid memory leaks.

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
 builtin/show-branch.c | 62 +++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..ad3a85fafa 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -34,13 +34,11 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
 
 static struct strvec default_args = STRVEC_INIT;
 
-/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
- */
 #define UNINTERESTING	01
 
+static unsigned int get_rev_flags(struct commit *commit);
+static void or_rev_flags(struct commit *commit, unsigned int flags);
+
 #define REV_SHIFT	 2
 #define MAX_REVS	(FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
 
@@ -64,7 +62,7 @@ static struct commit *interesting(struct prio_queue *queue)
 {
 	for (size_t i = 0; i < queue->nr; i++) {
 		struct commit *commit = queue->array[i].data;
-		if (commit->object.flags & UNINTERESTING)
+		if (get_rev_flags(commit) & UNINTERESTING)
 			continue;
 		return commit;
 	}
@@ -79,11 +77,25 @@ struct commit_name {
 define_commit_slab(commit_name_slab, struct commit_name *);
 static struct commit_name_slab name_slab;
 
+define_commit_slab(commit_rev_flags, unsigned int);
+static struct commit_rev_flags rev_flags_slab;
+
 static struct commit_name *commit_to_name(struct commit *commit)
 {
 	return *commit_name_slab_at(&name_slab, commit);
 }
 
+static unsigned int get_rev_flags(struct commit *commit)
+{
+	unsigned int *f = commit_rev_flags_peek(&rev_flags_slab, commit);
+	return f ? *f : 0;
+}
+
+static void or_rev_flags(struct commit *commit, unsigned int flags)
+{
+	*commit_rev_flags_at(&rev_flags_slab, commit) |= flags;
+}
+
 
 /* Name the commit as nth generation ancestor of head_name;
  * we count only the first-parent relationship for naming purposes.
@@ -215,7 +227,7 @@ static void name_commits(struct commit_list *list,
 
 static int mark_seen(struct commit *commit, struct commit_list **seen_p)
 {
-	if (!commit->object.flags) {
+	if (!get_rev_flags(commit)) {
 		commit_list_insert(commit, seen_p);
 		return 1;
 	}
@@ -234,7 +246,7 @@ static void join_revs(struct prio_queue *queue,
 		int still_interesting = !!interesting(queue);
 		struct commit *commit = prio_queue_peek(queue);
 		bool get_pending = true;
-		int flags = commit->object.flags & all_mask;
+		int flags = get_rev_flags(commit) & all_mask;
 
 		if (!still_interesting && extra <= 0)
 			break;
@@ -246,14 +258,14 @@ static void join_revs(struct prio_queue *queue,
 
 		while (parents) {
 			struct commit *p = parents->item;
-			int this_flag = p->object.flags;
+			int this_flag = get_rev_flags(p);
 			parents = parents->next;
 			if ((this_flag & flags) == flags)
 				continue;
 			repo_parse_commit(the_repository, p);
 			if (mark_seen(p, seen_p) && !still_interesting)
 				extra--;
-			p->object.flags |= flags;
+			or_rev_flags(p, flags);
 			if (get_pending)
 				prio_queue_replace(queue, p);
 			else
@@ -278,8 +290,8 @@ static void join_revs(struct prio_queue *queue,
 			struct commit *c = s->item;
 			struct commit_list *parents;
 
-			if (((c->object.flags & all_revs) != all_revs) &&
-			    !(c->object.flags & UNINTERESTING))
+			if (((get_rev_flags(c) & all_revs) != all_revs) &&
+			    !(get_rev_flags(c) & UNINTERESTING))
 				continue;
 
 			/* The current commit is either a merge base or
@@ -292,8 +304,8 @@ static void join_revs(struct prio_queue *queue,
 			while (parents) {
 				struct commit *p = parents->item;
 				parents = parents->next;
-				if (!(p->object.flags & UNINTERESTING)) {
-					p->object.flags |= UNINTERESTING;
+				if (!(get_rev_flags(p) & UNINTERESTING)) {
+					or_rev_flags(p, UNINTERESTING);
 					changed = 1;
 				}
 			}
@@ -517,12 +529,14 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)
 
 	for (const struct commit_list *s = seen; s; s = s->next) {
 		struct commit *commit = s->item;
-		int flags = commit->object.flags & all_mask;
+		int 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);
+
 		}
 	}
 	return exit_status;
@@ -538,9 +552,9 @@ static int show_independent(struct commit **rev,
 		struct commit *commit = rev[i];
 		unsigned int flag = rev_mask[i];
 
-		if (commit->object.flags == flag)
+		if (get_rev_flags(commit) == flag)
 			puts(oid_to_hex(&commit->object.oid));
-		commit->object.flags |= UNINTERESTING;
+		or_rev_flags(commit, UNINTERESTING);
 	}
 	return 0;
 }
@@ -607,7 +621,7 @@ 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);
 	for (i = count = 0; i < n; i++) {
 		if (flag & (1u << (i + REV_SHIFT)))
 			count++;
@@ -714,6 +728,7 @@ int cmd_show_branch(int ac,
 	int ret;
 
 	init_commit_name_slab(&name_slab);
+	init_commit_rev_flags(&rev_flags_slab);
 
 	repo_config(the_repository, git_show_branch_config, NULL);
 
@@ -889,13 +904,13 @@ int cmd_show_branch(int ac,
 		 * and so on.  REV_SHIFT bits from bit 0 are used for
 		 * internal bookkeeping.
 		 */
-		commit->object.flags |= flag;
-		if (commit->object.flags == flag)
+		or_rev_flags(commit, flag);
+		if (get_rev_flags(commit) == flag)
 			prio_queue_put(&queue, commit);
 		rev[num_rev] = commit;
 	}
 	for (i = 0; i < num_rev; i++)
-		rev_mask[i] = rev[i]->object.flags;
+		rev_mask[i] = get_rev_flags(rev[i]);
 
 	if (0 <= extra)
 		join_revs(&queue, &seen, num_rev, extra);
@@ -963,7 +978,7 @@ int cmd_show_branch(int ac,
 
 	for (struct commit_list *l = seen; l; l = l->next) {
 		struct commit *commit = l->item;
-		int this_flag = commit->object.flags;
+		int this_flag = get_rev_flags(commit);
 		int is_merge_point = ((this_flag & all_revs) == all_revs);
 
 		shown_merge_point |= is_merge_point;
@@ -1010,6 +1025,7 @@ int cmd_show_branch(int ac,
 		free(reflog_msg[i]);
 	commit_list_free(seen);
 	clear_prio_queue(&queue);
+	clear_commit_rev_flags(&rev_flags_slab);
 	free(args_copy);
 	free(head);
 	return ret;
-- 
2.54.0

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

[Gatla Vishweshwar Reddy]

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).

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.

Add helper functions get_rev_flags() and or_rev_flags() to
encapsulate slab access cleanly. Update all bit operations to use
UINT64_C(1) instead of 1u to ensure correct 64-bit shifts.
Initialize and clear the slab in cmd_show_branch() to avoid
memory leaks.

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---

Changes in v2:

- Use uint64_t instead of unsigned int for the slab element type.
  This lifts MAX_REVS from 27 to 62 branches since uint64_t provides
  64 bits instead of the 32 bits available in unsigned int.
- Update all bit shift operations from 1u to UINT64_C(1) to ensure
  correct 64-bit shifts without undefined behavior.
- Update printf format specifiers from %d to %zu for MAX_REVS since
  sizeof() expressions produce size_t, not int.

I noticed the prior RFC by Meet Soni (Feb 2025, Message-ID:
<20250217055024.3978-1-meetsoni3017@gmail.com>) which Junio C Hamano
and Jeff King reviewed. That patch did the basic conversion but did
not lift the MAX_REVS limitation. This v2 addresses Junio's feedback
where he suggested "using a slab whose element is still a bag of bits
that is wider than object.flags word is the most straight-forward way
to lift MAX_REVS limitation." We use uint64_t as that wider element.

 builtin/show-branch.c | 106 ++++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 45 deletions(-)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..625e456411 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -34,15 +34,13 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;

 static struct strvec default_args = STRVEC_INIT;

-/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
- */
 #define UNINTERESTING	01

+static uint64_t get_rev_flags(struct commit *commit);
+static void or_rev_flags(struct commit *commit, uint64_t flags);
+
 #define REV_SHIFT	 2
-#define MAX_REVS	(FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
+#define MAX_REVS	(sizeof(uint64_t) * 8 - REV_SHIFT)

 #define DEFAULT_REFLOG	4

@@ -64,7 +62,7 @@ static struct commit *interesting(struct prio_queue *queue)
 {
 	for (size_t i = 0; i < queue->nr; i++) {
 		struct commit *commit = queue->array[i].data;
-		if (commit->object.flags & UNINTERESTING)
+		if (get_rev_flags(commit) & UNINTERESTING)
 			continue;
 		return commit;
 	}
@@ -79,11 +77,25 @@ struct commit_name {
 define_commit_slab(commit_name_slab, struct commit_name *);
 static struct commit_name_slab name_slab;

+define_commit_slab(commit_rev_flags, uint64_t);
+static struct commit_rev_flags rev_flags_slab;
+
 static struct commit_name *commit_to_name(struct commit *commit)
 {
 	return *commit_name_slab_at(&name_slab, commit);
 }

+static uint64_t get_rev_flags(struct commit *commit)
+{
+	uint64_t *f = commit_rev_flags_peek(&rev_flags_slab, commit);
+	return f ? *f : 0;
+}
+
+static void or_rev_flags(struct commit *commit, uint64_t flags)
+{
+	*commit_rev_flags_at(&rev_flags_slab, commit) |= flags;
+}
+

 /* Name the commit as nth generation ancestor of head_name;
  * we count only the first-parent relationship for naming purposes.
@@ -215,7 +227,7 @@ static void name_commits(struct commit_list *list,

 static int mark_seen(struct commit *commit, struct commit_list **seen_p)
 {
-	if (!commit->object.flags) {
+	if (!get_rev_flags(commit)) {
 		commit_list_insert(commit, seen_p);
 		return 1;
 	}
@@ -226,15 +238,15 @@ static void join_revs(struct prio_queue *queue,
 		      struct commit_list **seen_p,
 		      int num_rev, int extra)
 {
-	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);

 	while (queue->nr) {
 		struct commit_list *parents;
 		int still_interesting = !!interesting(queue);
 		struct commit *commit = prio_queue_peek(queue);
 		bool get_pending = true;
-		int flags = commit->object.flags & all_mask;
+		uint64_t flags = get_rev_flags(commit) & all_mask;

 		if (!still_interesting && extra <= 0)
 			break;
@@ -246,14 +258,14 @@ static void join_revs(struct prio_queue *queue,

 		while (parents) {
 			struct commit *p = parents->item;
-			int this_flag = p->object.flags;
+			uint64_t this_flag = get_rev_flags(p);
 			parents = parents->next;
 			if ((this_flag & flags) == flags)
 				continue;
 			repo_parse_commit(the_repository, p);
 			if (mark_seen(p, seen_p) && !still_interesting)
 				extra--;
-			p->object.flags |= flags;
+			or_rev_flags(p, flags);
 			if (get_pending)
 				prio_queue_replace(queue, p);
 			else
@@ -278,8 +290,8 @@ static void join_revs(struct prio_queue *queue,
 			struct commit *c = s->item;
 			struct commit_list *parents;

-			if (((c->object.flags & all_revs) != all_revs) &&
-			    !(c->object.flags & UNINTERESTING))
+			if (((get_rev_flags(c) & all_revs) != all_revs) &&
+			    !(get_rev_flags(c) & UNINTERESTING))
 				continue;

 			/* The current commit is either a merge base or
@@ -292,8 +304,8 @@ static void join_revs(struct prio_queue *queue,
 			while (parents) {
 				struct commit *p = parents->item;
 				parents = parents->next;
-				if (!(p->object.flags & UNINTERESTING)) {
-					p->object.flags |= UNINTERESTING;
+				if (!(get_rev_flags(p) & UNINTERESTING)) {
+					or_rev_flags(p, UNINTERESTING);
 					changed = 1;
 				}
 			}
@@ -410,8 +422,8 @@ static int append_ref(const char *refname, const struct object_id *oid,
 				return 0;
 	}
 	if (MAX_REVS <= ref_name_cnt) {
-		warning(Q_("ignoring %s; cannot handle more than %d ref",
-			   "ignoring %s; cannot handle more than %d refs",
+		warning(Q_("ignoring %s; cannot handle more than %zu ref",
+			   "ignoring %s; cannot handle more than %zu refs",
 			   MAX_REVS), refname, MAX_REVS);
 		return 0;
 	}
@@ -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);
+
 		}
 	}
 	return exit_status;
@@ -530,17 +544,17 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)

 static int show_independent(struct commit **rev,
 			    int num_rev,
-			    unsigned int *rev_mask)
+			    uint64_t *rev_mask)
 {
 	int i;

 	for (i = 0; i < num_rev; i++) {
 		struct commit *commit = rev[i];
-		unsigned int flag = rev_mask[i];
+		uint64_t flag = rev_mask[i];

-		if (commit->object.flags == flag)
+		if (get_rev_flags(commit) == flag)
 			puts(oid_to_hex(&commit->object.oid));
-		commit->object.flags |= UNINTERESTING;
+		or_rev_flags(commit, UNINTERESTING);
 	}
 	return 0;
 }
@@ -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);
 	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)
@@ -648,10 +662,10 @@ int cmd_show_branch(int ac,
 	char *reflog_msg[MAX_REVS] = {0};
 	struct commit_list *seen = NULL;
 	struct prio_queue queue = { compare_commits_by_commit_date };
-	unsigned int rev_mask[MAX_REVS];
+	uint64_t rev_mask[MAX_REVS];
 	int num_rev, i, extra = 0;
 	int all_heads = 0, all_remotes = 0;
-	int all_mask, all_revs;
+	uint64_t all_mask, all_revs;
 	enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
 	char *head;
 	struct object_id head_oid;
@@ -714,6 +728,7 @@ int cmd_show_branch(int ac,
 	int ret;

 	init_commit_name_slab(&name_slab);
+	init_commit_rev_flags(&rev_flags_slab);

 	repo_config(the_repository, git_show_branch_config, NULL);

@@ -759,7 +774,7 @@ int cmd_show_branch(int ac,
 		struct object_id oid;
 		char *ref;
 		int base = 0;
-		unsigned int flags = 0;
+		uint64_t flags = 0;

 		if (ac == 0) {
 			static const char *fake_av[2];
@@ -779,8 +794,8 @@ int cmd_show_branch(int ac,
 			die(_("--reflog option needs one branch name"));

 		if (MAX_REVS < reflog)
-			die(Q_("only %d entry can be shown at one time.",
-			       "only %d entries can be shown at one time.",
+			die(Q_("only %zu entry can be shown at one time.",
+			       "only %zu entries can be shown at one time.",
 			       MAX_REVS), MAX_REVS);
 		if (!repo_dwim_ref(the_repository, *av, strlen(*av), &oid,
 				   &ref, 0))
@@ -870,11 +885,11 @@ int cmd_show_branch(int ac,

 	for (num_rev = 0; ref_name[num_rev]; num_rev++) {
 		struct object_id revkey;
-		unsigned int flag = 1u << (num_rev + REV_SHIFT);
+		uint64_t flag = UINT64_C(1) << (num_rev + REV_SHIFT);

 		if (MAX_REVS <= num_rev)
-			die(Q_("cannot handle more than %d rev.",
-			       "cannot handle more than %d revs.",
+			die(Q_("cannot handle more than %zu rev.",
+			       "cannot handle more than %zu revs.",
 			       MAX_REVS), MAX_REVS);
 		if (repo_get_oid(the_repository, ref_name[num_rev], &revkey))
 			die(_("'%s' is not a valid ref."), ref_name[num_rev]);
@@ -889,13 +904,13 @@ int cmd_show_branch(int ac,
 		 * and so on.  REV_SHIFT bits from bit 0 are used for
 		 * internal bookkeeping.
 		 */
-		commit->object.flags |= flag;
-		if (commit->object.flags == flag)
+		or_rev_flags(commit, flag);
+		if (get_rev_flags(commit) == flag)
 			prio_queue_put(&queue, commit);
 		rev[num_rev] = commit;
 	}
 	for (i = 0; i < num_rev; i++)
-		rev_mask[i] = rev[i]->object.flags;
+		rev_mask[i] = get_rev_flags(rev[i]);

 	if (0 <= extra)
 		join_revs(&queue, &seen, num_rev, extra);
@@ -958,12 +973,12 @@ int cmd_show_branch(int ac,
 	if (!sha1_name && !no_name)
 		name_commits(seen, rev, ref_name, num_rev);

-	all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
-	all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
+	all_mask = ((UINT64_C(1) << (REV_SHIFT + num_rev)) - 1);
+	all_revs = all_mask & ~((UINT64_C(1) << REV_SHIFT) - 1);

 	for (struct commit_list *l = seen; l; l = l->next) {
 		struct commit *commit = l->item;
-		int this_flag = commit->object.flags;
+		uint64_t this_flag = get_rev_flags(commit);
 		int is_merge_point = ((this_flag & all_revs) == all_revs);

 		shown_merge_point |= is_merge_point;
@@ -973,14 +988,14 @@ int cmd_show_branch(int ac,
 					  commit->parents->next);
 			if (topics &&
 			    !is_merge_point &&
-			    (this_flag & (1u << REV_SHIFT)))
+			    (this_flag & (UINT64_C(1) << REV_SHIFT)))
 				continue;
 			if (!sparse && is_merge &&
 			    omit_in_dense(commit, rev, num_rev))
 				continue;
 			for (i = 0; i < num_rev; i++) {
 				int mark;
-				if (!(this_flag & (1u << (i + REV_SHIFT))))
+				if (!(this_flag & (UINT64_C(1) << (i + REV_SHIFT))))
 					mark = ' ';
 				else if (is_merge)
 					mark = '-';
@@ -1010,6 +1025,7 @@ int cmd_show_branch(int ac,
 		free(reflog_msg[i]);
 	commit_list_free(seen);
 	clear_prio_queue(&queue);
+	clear_commit_rev_flags(&rev_flags_slab);
 	free(args_copy);
 	free(head);
 	return ret;
--
2.54.0

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

[Junio C Hamano]

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

> 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).

> 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.

> @@ -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?

> @@ -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.

>  	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)

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

[Jeff King]

On Tue, Jul 14, 2026 at 01:41:15PM -0700, Junio C Hamano wrote:

> 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).

Yep. Going back to the last time this topic came up, I'll just point at:

  https://lore.kernel.org/git/20250225011757.GA752084@coredump.intra.peff.net/

which references one of the earliest commit-slab series. Especially the
part that adds arbitrary-sized bitset support, which could be useful
here (patch 4 adds the bitset, patch 6 shows how it is used).

-Peff

[PATCH v3] show-branch: convert per-branch flags to commit-slab

[Gatla Vishweshwar Reddy]

show-branch uses commit->object.flags to store per-branch
reachability bits, one bit per branch starting at REV_SHIFT.
The flags word has only a fixed number of available bits, limiting
the number of branches that can be shown simultaneously to MAX_REVS.

Convert the per-branch bits to a dedicated commit-slab using uint64_t
as the element type, initialized with a stride via
init_commit_rev_flags_with_stride(). Keep the UNINTERESTING bit in
object.flags where it belongs, as it is used for revision walking and
does not need to be in the per-branch slab. With UNINTERESTING removed
from the slab, REV_SHIFT becomes 0 and all 64 bits of uint64_t are
available for branch tracking, lifting MAX_REVS from 27 to 64 branches.

Add helper functions get_rev_flags_ptr(), peek_rev_flags_ptr(),
has_any_rev_flags(), or_rev_flag_bit(), test_rev_flag_bit(), and
has_all_rev_flags() to encapsulate per-bit slab access cleanly.
Update all bit operations to use UINT64_C(1) for correct 64-bit shifts.
Initialize and clear the slab in cmd_show_branch().

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---

Changes in v3:
- Keep UNINTERESTING in object.flags as suggested by Junio
- Slab stores only per-branch bits with REV_SHIFT=0
- All 64 bits of uint64_t available for branches, MAX_REVS=64
- Fix uint64_t flag in omit_in_dense() (was int)
- Fix indentation in show_merge_base()
- Replace all_mask/all_revs with has_all_rev_flags() helper
- Use UINT64_C(1) for all bit shifts

In response to Junio:
- UNINTERESTING kept in object.flags; slab is per-branch bits only

In response to Jeff King:
- init_commit_rev_flags_with_stride() is used as foundation.
  Current stride=1 gives 64 branches. Dynamic stride for >64
  branches can be added as a follow-up.

 builtin/show-branch.c | 143 ++++++++++++++++++++++++------------------
 1 file changed, 83 insertions(+), 60 deletions(-)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..70436007ec 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -34,16 +34,9 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;

 static struct strvec default_args = STRVEC_INIT;

-/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
- */
 #define UNINTERESTING	01
-
-#define REV_SHIFT	 2
-#define MAX_REVS	(FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
-
+#define REV_SHIFT	 0
+#define MAX_REVS	(sizeof(uint64_t) * 8)
 #define DEFAULT_REFLOG	4

 static const char *get_color_code(int idx)
@@ -79,11 +72,56 @@ struct commit_name {
 define_commit_slab(commit_name_slab, struct commit_name *);
 static struct commit_name_slab name_slab;

+define_commit_slab(commit_rev_flags, uint64_t);
+static struct commit_rev_flags rev_flags_slab;
+static int flags_stride; /* number of uint64_t words per commit */
+
 static struct commit_name *commit_to_name(struct commit *commit)
 {
 	return *commit_name_slab_at(&name_slab, commit);
 }

+static uint64_t *get_rev_flags_ptr(struct commit *commit)
+{
+	return commit_rev_flags_at(&rev_flags_slab, commit);
+}
+
+static uint64_t *peek_rev_flags_ptr(struct commit *commit)
+{
+	return commit_rev_flags_peek(&rev_flags_slab, commit);
+}
+
+static int has_any_rev_flags(struct commit *commit)
+{
+	uint64_t *f = peek_rev_flags_ptr(commit);
+	int i;
+	if (!f)
+		return 0;
+	for (i = 0; i < flags_stride; i++)
+		if (f[i])
+			return 1;
+	return 0;
+}
+
+static void or_rev_flag_bit(struct commit *commit, int branch)
+{
+	get_rev_flags_ptr(commit)[branch / 64] |= UINT64_C(1) << (branch % 64);
+}
+
+static int test_rev_flag_bit(struct commit *commit, int branch)
+{
+	uint64_t *f = peek_rev_flags_ptr(commit);
+	return f && !!(f[branch / 64] & (UINT64_C(1) << (branch % 64)));
+}
+
+static int has_all_rev_flags(struct commit *commit, int num_rev)
+{
+	int i;
+	for (i = 0; i < num_rev; i++)
+		if (!test_rev_flag_bit(commit, i))
+			return 0;
+	return 1;
+}

 /* Name the commit as nth generation ancestor of head_name;
  * we count only the first-parent relationship for naming purposes.
@@ -215,7 +253,7 @@ static void name_commits(struct commit_list *list,

 static int mark_seen(struct commit *commit, struct commit_list **seen_p)
 {
-	if (!commit->object.flags) {
+	if (!has_any_rev_flags(commit)) {
 		commit_list_insert(commit, seen_p);
 		return 1;
 	}
@@ -226,34 +264,34 @@ static void join_revs(struct prio_queue *queue,
 		      struct commit_list **seen_p,
 		      int num_rev, int extra)
 {
-	int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
-	int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
 	while (queue->nr) {
 		struct commit_list *parents;
 		int still_interesting = !!interesting(queue);
 		struct commit *commit = prio_queue_peek(queue);
 		bool get_pending = true;
-		int flags = commit->object.flags & all_mask;

 		if (!still_interesting && extra <= 0)
 			break;

 		mark_seen(commit, seen_p);
-		if ((flags & all_revs) == all_revs)
-			flags |= UNINTERESTING;
+		if (has_all_rev_flags(commit, num_rev))
+			commit->object.flags |= UNINTERESTING;
 		parents = commit->parents;

 		while (parents) {
 			struct commit *p = parents->item;
-			int this_flag = p->object.flags;
 			parents = parents->next;
-			if ((this_flag & flags) == flags)
+			if (has_all_rev_flags(p, num_rev))
 				continue;
 			repo_parse_commit(the_repository, p);
 			if (mark_seen(p, seen_p) && !still_interesting)
 				extra--;
-			p->object.flags |= flags;
+			{
+				int _b;
+				for (_b = 0; _b < num_rev; _b++)
+					if (test_rev_flag_bit(commit, _b))
+						or_rev_flag_bit(p, _b);
+			}
 			if (get_pending)
 				prio_queue_replace(queue, p);
 			else
@@ -263,7 +301,6 @@ static void join_revs(struct prio_queue *queue,
 		if (get_pending)
 			prio_queue_get(queue);
 	}
-
 	/*
 	 * Postprocess to complete well-poisoning.
 	 *
@@ -278,7 +315,7 @@ static void join_revs(struct prio_queue *queue,
 			struct commit *c = s->item;
 			struct commit_list *parents;

-			if (((c->object.flags & all_revs) != all_revs) &&
+			if (!has_all_rev_flags(c, num_rev) &&
 			    !(c->object.flags & UNINTERESTING))
 				continue;

@@ -410,8 +447,8 @@ static int append_ref(const char *refname, const struct object_id *oid,
 				return 0;
 	}
 	if (MAX_REVS <= ref_name_cnt) {
-		warning(Q_("ignoring %s; cannot handle more than %d ref",
-			   "ignoring %s; cannot handle more than %d refs",
+		warning(Q_("ignoring %s; cannot handle more than %zu ref",
+			   "ignoring %s; cannot handle more than %zu refs",
 			   MAX_REVS), refname, MAX_REVS);
 		return 0;
 	}
@@ -511,15 +548,12 @@ 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);
 	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;
-		if (!(flags & UNINTERESTING) &&
-		    ((flags & all_revs) == all_revs)) {
+		if (!(commit->object.flags & UNINTERESTING) &&
+			has_all_rev_flags(commit, num_rev)) {
 			puts(oid_to_hex(&commit->object.oid));
 			exit_status = 0;
 			commit->object.flags |= UNINTERESTING;
@@ -528,17 +562,13 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)
 	return exit_status;
 }

-static int show_independent(struct commit **rev,
-			    int num_rev,
-			    unsigned int *rev_mask)
+static int show_independent(struct commit **rev, int num_rev)
 {
 	int i;

 	for (i = 0; i < num_rev; i++) {
 		struct commit *commit = rev[i];
-		unsigned int flag = rev_mask[i];
-
-		if (commit->object.flags == flag)
+		if (test_rev_flag_bit(commit, i))
 			puts(oid_to_hex(&commit->object.oid));
 		commit->object.flags |= UNINTERESTING;
 	}
@@ -603,13 +633,12 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
 	 * Otherwise, if it is a merge that is reachable from only one
 	 * tip, it is not that interesting.
 	 */
-	int i, flag, count;
+	int i, count;
 	for (i = 0; i < n; i++)
 		if (rev[i] == commit)
 			return 0;
-	flag = commit->object.flags;
 	for (i = count = 0; i < n; i++) {
-		if (flag & (1u << (i + REV_SHIFT)))
+		if (test_rev_flag_bit(commit, i))
 			count++;
 	}
 	if (count == 1)
@@ -648,10 +677,8 @@ int cmd_show_branch(int ac,
 	char *reflog_msg[MAX_REVS] = {0};
 	struct commit_list *seen = NULL;
 	struct prio_queue queue = { compare_commits_by_commit_date };
-	unsigned int rev_mask[MAX_REVS];
 	int num_rev, i, extra = 0;
 	int all_heads = 0, all_remotes = 0;
-	int all_mask, all_revs;
 	enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
 	char *head;
 	struct object_id head_oid;
@@ -713,7 +740,8 @@ int cmd_show_branch(int ac,
 	const char **args_copy = NULL;
 	int ret;

-	init_commit_name_slab(&name_slab);
+	flags_stride = (MAX_REVS + 63) / 64;
+	init_commit_rev_flags_with_stride(&rev_flags_slab, flags_stride);

 	repo_config(the_repository, git_show_branch_config, NULL);

@@ -779,8 +807,8 @@ int cmd_show_branch(int ac,
 			die(_("--reflog option needs one branch name"));

 		if (MAX_REVS < reflog)
-			die(Q_("only %d entry can be shown at one time.",
-			       "only %d entries can be shown at one time.",
+			die(Q_("only %zu entry can be shown at one time.",
+			       "only %zu entries can be shown at one time.",
 			       MAX_REVS), MAX_REVS);
 		if (!repo_dwim_ref(the_repository, *av, strlen(*av), &oid,
 				   &ref, 0))
@@ -870,11 +898,11 @@ int cmd_show_branch(int ac,

 	for (num_rev = 0; ref_name[num_rev]; num_rev++) {
 		struct object_id revkey;
-		unsigned int flag = 1u << (num_rev + REV_SHIFT);
+		int first_seen;

 		if (MAX_REVS <= num_rev)
-			die(Q_("cannot handle more than %d rev.",
-			       "cannot handle more than %d revs.",
+			die(Q_("cannot handle more than %zu rev.",
+			       "cannot handle more than %zu revs.",
 			       MAX_REVS), MAX_REVS);
 		if (repo_get_oid(the_repository, ref_name[num_rev], &revkey))
 			die(_("'%s' is not a valid ref."), ref_name[num_rev]);
@@ -885,17 +913,15 @@ int cmd_show_branch(int ac,
 		repo_parse_commit(the_repository, commit);
 		mark_seen(commit, &seen);

-		/* rev#0 uses bit REV_SHIFT, rev#1 uses bit REV_SHIFT+1,
-		 * and so on.  REV_SHIFT bits from bit 0 are used for
-		 * internal bookkeeping.
+		/* rev#0 uses bit 0, rev#1 uses bit 1,
+		 * and so on.  All bits are available for branch tracking.
 		 */
-		commit->object.flags |= flag;
-		if (commit->object.flags == flag)
+		first_seen = !has_any_rev_flags(commit);
+		or_rev_flag_bit(commit, num_rev);
+		if (first_seen)
 			prio_queue_put(&queue, commit);
 		rev[num_rev] = commit;
 	}
-	for (i = 0; i < num_rev; i++)
-		rev_mask[i] = rev[i]->object.flags;

 	if (0 <= extra)
 		join_revs(&queue, &seen, num_rev, extra);
@@ -908,7 +934,7 @@ int cmd_show_branch(int ac,
 	}

 	if (independent) {
-		ret = show_independent(rev, num_rev, rev_mask);
+		ret = show_independent(rev, num_rev);
 		goto out;
 	}

@@ -958,13 +984,9 @@ int cmd_show_branch(int ac,
 	if (!sha1_name && !no_name)
 		name_commits(seen, rev, ref_name, num_rev);

-	all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
-	all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
 	for (struct commit_list *l = seen; l; l = l->next) {
 		struct commit *commit = l->item;
-		int this_flag = commit->object.flags;
-		int is_merge_point = ((this_flag & all_revs) == all_revs);
+		int is_merge_point = has_all_rev_flags(commit, num_rev);

 		shown_merge_point |= is_merge_point;

@@ -973,14 +995,14 @@ int cmd_show_branch(int ac,
 					  commit->parents->next);
 			if (topics &&
 			    !is_merge_point &&
-			    (this_flag & (1u << REV_SHIFT)))
+			    test_rev_flag_bit(commit, 0))
 				continue;
 			if (!sparse && is_merge &&
 			    omit_in_dense(commit, rev, num_rev))
 				continue;
 			for (i = 0; i < num_rev; i++) {
 				int mark;
-				if (!(this_flag & (1u << (i + REV_SHIFT))))
+				if (!test_rev_flag_bit(commit, i))
 					mark = ' ';
 				else if (is_merge)
 					mark = '-';
@@ -1010,6 +1032,7 @@ int cmd_show_branch(int ac,
 		free(reflog_msg[i]);
 	commit_list_free(seen);
 	clear_prio_queue(&queue);
+	clear_commit_rev_flags(&rev_flags_slab);
 	free(args_copy);
 	free(head);
 	return ret;
--
2.54.0

Re: [PATCH v3] show-branch: convert per-branch flags to commit-slab

[Junio C Hamano]

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

> +static int show_independent(struct commit **rev, int num_rev)
>  {
>  	int i;
>
>  	for (i = 0; i < num_rev; i++) {
>  		struct commit *commit = rev[i];
> -		unsigned int flag = rev_mask[i];
> -
> -		if (commit->object.flags == flag)
> +		if (test_rev_flag_bit(commit, i))
>  			puts(oid_to_hex(&commit->object.oid));
>  		commit->object.flags |= UNINTERESTING;
>  	}

These two perform different actions, do they not?  The original code
insists that the commit is reachable from only one tip (i.e., that
the commit's flag word has only a single bit set, corresponding to
the i-th revision).  This is why the implementation does not use:

		if (commit->object.flags & flag)

By contrast, the updated version merely checks whether the bit for
the i-th revision is set, without verifying that all other bits are
cleared.

Or am I misreading the patch?

Thanks.

[PATCH v4] show-branch: convert per-branch flags to commit-slab

[Gatla Vishweshwar Reddy]

show-branch uses commit->object.flags to store per-branch
reachability bits, one bit per branch starting at REV_SHIFT.
The flags word has only a fixed number of available bits, limiting
the number of branches that can be shown simultaneously to MAX_REVS.

Convert the per-branch bits to a dedicated commit-slab using uint64_t
as the element type, initialized with a stride via
init_commit_rev_flags_with_stride(). Keep the UNINTERESTING bit in
object.flags where it belongs, as it is used for revision walking and
does not need to be in the per-branch slab. With UNINTERESTING removed
from the slab, REV_SHIFT becomes 0 and all 64 bits of uint64_t are
available for branch tracking, lifting MAX_REVS from 27 to 64 branches.

Add helper functions get_rev_flags_ptr(), peek_rev_flags_ptr(),
has_any_rev_flags(), or_rev_flag_bit(), test_rev_flag_bit(),
has_all_rev_flags(), and has_only_rev_flag_bit() to encapsulate
per-bit slab access cleanly. Use has_only_rev_flag_bit() in
show_independent() to preserve the original semantics: a commit is
independent only if reachable from exactly one tip, not merely if
the i-th bit happens to be set. Update all bit operations to use
UINT64_C(1) for correct 64-bit shifts. Initialize and clear the slab
in cmd_show_branch().

Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---

Changes in v4:
- Fix show_independent() to use has_only_rev_flag_bit() instead of
  test_rev_flag_bit(), preserving the original semantics: a commit is
  independent only if reachable from exactly one tip, not merely if
  the i-th bit is set.
  
 builtin/show-branch.c | 159 ++++++++++++++++++++++++++----------------
 1 file changed, 99 insertions(+), 60 deletions(-)

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..a6598541b9 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -34,16 +34,9 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
 
 static struct strvec default_args = STRVEC_INIT;
 
-/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
- */
 #define UNINTERESTING	01
-
-#define REV_SHIFT	 2
-#define MAX_REVS	(FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
-
+#define REV_SHIFT	 0
+#define MAX_REVS	(sizeof(uint64_t) * 8)
 #define DEFAULT_REFLOG	4
 
 static const char *get_color_code(int idx)
@@ -79,11 +72,72 @@ struct commit_name {
 define_commit_slab(commit_name_slab, struct commit_name *);
 static struct commit_name_slab name_slab;
 
+define_commit_slab(commit_rev_flags, uint64_t);
+static struct commit_rev_flags rev_flags_slab;
+static int flags_stride; /* number of uint64_t words per commit */
+
 static struct commit_name *commit_to_name(struct commit *commit)
 {
 	return *commit_name_slab_at(&name_slab, commit);
 }
 
+static uint64_t *get_rev_flags_ptr(struct commit *commit)
+{
+	return commit_rev_flags_at(&rev_flags_slab, commit);
+}
+
+static uint64_t *peek_rev_flags_ptr(struct commit *commit)
+{
+	return commit_rev_flags_peek(&rev_flags_slab, commit);
+}
+
+static int has_any_rev_flags(struct commit *commit)
+{
+	uint64_t *f = peek_rev_flags_ptr(commit);
+	int i;
+	if (!f)
+		return 0;
+	for (i = 0; i < flags_stride; i++)
+		if (f[i])
+			return 1;
+	return 0;
+}
+
+static void or_rev_flag_bit(struct commit *commit, int branch)
+{
+	get_rev_flags_ptr(commit)[branch / 64] |= UINT64_C(1) << (branch % 64);
+}
+
+static int test_rev_flag_bit(struct commit *commit, int branch)
+{
+	uint64_t *f = peek_rev_flags_ptr(commit);
+	return f && !!(f[branch / 64] & (UINT64_C(1) << (branch % 64)));
+}
+
+static int has_all_rev_flags(struct commit *commit, int num_rev)
+{
+	int i;
+	for (i = 0; i < num_rev; i++)
+		if (!test_rev_flag_bit(commit, i))
+			return 0;
+	return 1;
+}
+
+static int has_only_rev_flag_bit(struct commit *commit, int branch)
+{
+	uint64_t *f = peek_rev_flags_ptr(commit);
+	int i;
+	if (!f)
+		return 0;
+	for (i = 0; i < flags_stride; i++) {
+		uint64_t expected = (i == branch / 64)
+				    ? (UINT64_C(1) << (branch % 64))
+				    : 0;
+		if (f[i] != expected)
+			return 0;
+	}
+	return 1;
+}
 
 /* Name the commit as nth generation ancestor of head_name;
  * we count only the first-parent relationship for naming purposes.
@@ -215,7 +269,7 @@ static void name_commits(struct commit_list *list,
 
 static int mark_seen(struct commit *commit, struct commit_list **seen_p)
 {
-	if (!commit->object.flags) {
+	if (!has_any_rev_flags(commit)) {
 		commit_list_insert(commit, seen_p);
 		return 1;
 	}
@@ -226,34 +280,34 @@ static void join_revs(struct prio_queue *queue,
 		      struct commit_list **seen_p,
 		      int num_rev, int extra)
 {
-	int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
-	int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
 	while (queue->nr) {
 		struct commit_list *parents;
 		int still_interesting = !!interesting(queue);
 		struct commit *commit = prio_queue_peek(queue);
 		bool get_pending = true;
-		int flags = commit->object.flags & all_mask;
 
 		if (!still_interesting && extra <= 0)
 			break;
 
 		mark_seen(commit, seen_p);
-		if ((flags & all_revs) == all_revs)
-			flags |= UNINTERESTING;
+		if (has_all_rev_flags(commit, num_rev))
+			commit->object.flags |= UNINTERESTING;
 		parents = commit->parents;
 
 		while (parents) {
 			struct commit *p = parents->item;
-			int this_flag = p->object.flags;
 			parents = parents->next;
-			if ((this_flag & flags) == flags)
+			if (has_all_rev_flags(p, num_rev))
 				continue;
 			repo_parse_commit(the_repository, p);
 			if (mark_seen(p, seen_p) && !still_interesting)
 				extra--;
-			p->object.flags |= flags;
+			{
+				int _b;
+				for (_b = 0; _b < num_rev; _b++)
+					if (test_rev_flag_bit(commit, _b))
+						or_rev_flag_bit(p, _b);
+			}
 			if (get_pending)
 				prio_queue_replace(queue, p);
 			else
@@ -263,7 +317,6 @@ static void join_revs(struct prio_queue *queue,
 		if (get_pending)
 			prio_queue_get(queue);
 	}
-
 	/*
 	 * Postprocess to complete well-poisoning.
 	 *
@@ -278,7 +331,7 @@ static void join_revs(struct prio_queue *queue,
 			struct commit *c = s->item;
 			struct commit_list *parents;
 
-			if (((c->object.flags & all_revs) != all_revs) &&
+			if (!has_all_rev_flags(c, num_rev) &&
 			    !(c->object.flags & UNINTERESTING))
 				continue;
 
@@ -410,8 +463,8 @@ static int append_ref(const char *refname, const struct object_id *oid,
 				return 0;
 	}
 	if (MAX_REVS <= ref_name_cnt) {
-		warning(Q_("ignoring %s; cannot handle more than %d ref",
-			   "ignoring %s; cannot handle more than %d refs",
+		warning(Q_("ignoring %s; cannot handle more than %zu ref",
+			   "ignoring %s; cannot handle more than %zu refs",
 			   MAX_REVS), refname, MAX_REVS);
 		return 0;
 	}
@@ -511,15 +564,12 @@ 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);
 	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;
-		if (!(flags & UNINTERESTING) &&
-		    ((flags & all_revs) == all_revs)) {
+		if (!(commit->object.flags & UNINTERESTING) &&
+			has_all_rev_flags(commit, num_rev)) {
 			puts(oid_to_hex(&commit->object.oid));
 			exit_status = 0;
 			commit->object.flags |= UNINTERESTING;
@@ -528,17 +578,13 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)
 	return exit_status;
 }
 
-static int show_independent(struct commit **rev,
-			    int num_rev,
-			    unsigned int *rev_mask)
+static int show_independent(struct commit **rev, int num_rev)
 {
 	int i;
 
 	for (i = 0; i < num_rev; i++) {
 		struct commit *commit = rev[i];
-		unsigned int flag = rev_mask[i];
-
-		if (commit->object.flags == flag)
+		if (has_only_rev_flag_bit(commit, i))
 			puts(oid_to_hex(&commit->object.oid));
 		commit->object.flags |= UNINTERESTING;
 	}
@@ -603,13 +649,12 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
 	 * Otherwise, if it is a merge that is reachable from only one
 	 * tip, it is not that interesting.
 	 */
-	int i, flag, count;
+	int i, count;
 	for (i = 0; i < n; i++)
 		if (rev[i] == commit)
 			return 0;
-	flag = commit->object.flags;
 	for (i = count = 0; i < n; i++) {
-		if (flag & (1u << (i + REV_SHIFT)))
+		if (test_rev_flag_bit(commit, i))
 			count++;
 	}
 	if (count == 1)
@@ -648,10 +693,8 @@ int cmd_show_branch(int ac,
 	char *reflog_msg[MAX_REVS] = {0};
 	struct commit_list *seen = NULL;
 	struct prio_queue queue = { compare_commits_by_commit_date };
-	unsigned int rev_mask[MAX_REVS];
 	int num_rev, i, extra = 0;
 	int all_heads = 0, all_remotes = 0;
-	int all_mask, all_revs;
 	enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
 	char *head;
 	struct object_id head_oid;
@@ -713,7 +756,8 @@ int cmd_show_branch(int ac,
 	const char **args_copy = NULL;
 	int ret;
 
-	init_commit_name_slab(&name_slab);
+	flags_stride = (MAX_REVS + 63) / 64;
+	init_commit_rev_flags_with_stride(&rev_flags_slab, flags_stride);
 
 	repo_config(the_repository, git_show_branch_config, NULL);
 
@@ -779,8 +823,8 @@ int cmd_show_branch(int ac,
 			die(_("--reflog option needs one branch name"));
 
 		if (MAX_REVS < reflog)
-			die(Q_("only %d entry can be shown at one time.",
-			       "only %d entries can be shown at one time.",
+			die(Q_("only %zu entry can be shown at one time.",
+			       "only %zu entries can be shown at one time.",
 			       MAX_REVS), MAX_REVS);
 		if (!repo_dwim_ref(the_repository, *av, strlen(*av), &oid,
 				   &ref, 0))
@@ -870,11 +914,11 @@ int cmd_show_branch(int ac,
 
 	for (num_rev = 0; ref_name[num_rev]; num_rev++) {
 		struct object_id revkey;
-		unsigned int flag = 1u << (num_rev + REV_SHIFT);
+		int first_seen;
 
 		if (MAX_REVS <= num_rev)
-			die(Q_("cannot handle more than %d rev.",
-			       "cannot handle more than %d revs.",
+			die(Q_("cannot handle more than %zu rev.",
+			       "cannot handle more than %zu revs.",
 			       MAX_REVS), MAX_REVS);
 		if (repo_get_oid(the_repository, ref_name[num_rev], &revkey))
 			die(_("'%s' is not a valid ref."), ref_name[num_rev]);
@@ -885,17 +929,15 @@ int cmd_show_branch(int ac,
 		repo_parse_commit(the_repository, commit);
 		mark_seen(commit, &seen);
 
-		/* rev#0 uses bit REV_SHIFT, rev#1 uses bit REV_SHIFT+1,
-		 * and so on.  REV_SHIFT bits from bit 0 are used for
-		 * internal bookkeeping.
+		/* rev#0 uses bit 0, rev#1 uses bit 1,
+		 * and so on.  All bits are available for branch tracking.
 		 */
-		commit->object.flags |= flag;
-		if (commit->object.flags == flag)
+		first_seen = !has_any_rev_flags(commit);
+		or_rev_flag_bit(commit, num_rev);
+		if (first_seen)
 			prio_queue_put(&queue, commit);
 		rev[num_rev] = commit;
 	}
-	for (i = 0; i < num_rev; i++)
-		rev_mask[i] = rev[i]->object.flags;
 
 	if (0 <= extra)
 		join_revs(&queue, &seen, num_rev, extra);
@@ -908,7 +950,7 @@ int cmd_show_branch(int ac,
 	}
 
 	if (independent) {
-		ret = show_independent(rev, num_rev, rev_mask);
+		ret = show_independent(rev, num_rev);
 		goto out;
 	}
 
@@ -958,13 +1000,9 @@ int cmd_show_branch(int ac,
 	if (!sha1_name && !no_name)
 		name_commits(seen, rev, ref_name, num_rev);
 
-	all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
-	all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
 	for (struct commit_list *l = seen; l; l = l->next) {
 		struct commit *commit = l->item;
-		int this_flag = commit->object.flags;
-		int is_merge_point = ((this_flag & all_revs) == all_revs);
+		int is_merge_point = has_all_rev_flags(commit, num_rev);
 
 		shown_merge_point |= is_merge_point;
 
@@ -973,14 +1011,14 @@ int cmd_show_branch(int ac,
 					  commit->parents->next);
 			if (topics &&
 			    !is_merge_point &&
-			    (this_flag & (1u << REV_SHIFT)))
+			    test_rev_flag_bit(commit, 0))
 				continue;
 			if (!sparse && is_merge &&
 			    omit_in_dense(commit, rev, num_rev))
 				continue;
 			for (i = 0; i < num_rev; i++) {
 				int mark;
-				if (!(this_flag & (1u << (i + REV_SHIFT))))
+				if (!test_rev_flag_bit(commit, i))
 					mark = ' ';
 				else if (is_merge)
 					mark = '-';
@@ -1010,6 +1048,7 @@ int cmd_show_branch(int ac,
 		free(reflog_msg[i]);
 	commit_list_free(seen);
 	clear_prio_queue(&queue);
+	clear_commit_rev_flags(&rev_flags_slab);
 	free(args_copy);
 	free(head);
 	return ret;
-- 
2.54.0

Re: [PATCH v4] show-branch: convert per-branch flags to commit-slab

[Patrick Steinhardt]

On Wed, Jul 15, 2026 at 09:48:56AM +0530, Gatla Vishweshwar Reddy wrote:
> Changes in v4:
> - Fix show_independent() to use has_only_rev_flag_bit() instead of
>   test_rev_flag_bit(), preserving the original semantics: a commit is
>   independent only if reachable from exactly one tip, not merely if
>   the i-th bit is set.

Please note that it's considered good etiquette on our mailing list to
not only post new versions of a patch series, but to also reply to at
least some of the review comments you got [1]. This makes the reviewer
feel like they're not only talking to a code producing entity (read: AI
prompt), but rather to a human on the other side of the internet.

Thanks!

Patrick

[1]: https://git-scm.com/docs/MyFirstContribution#reviewing

Re: [PATCH v3] show-branch: convert per-branch flags to commit-slab

[Junio C Hamano]

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

> In response to Jeff King:
> - init_commit_rev_flags_with_stride() is used as foundation.
>   Current stride=1 gives 64 branches. Dynamic stride for >64
>   branches can be added as a follow-up.

If that is the case ...

>  builtin/show-branch.c | 143 ++++++++++++++++++++++++------------------
>  1 file changed, 83 insertions(+), 60 deletions(-)
>
> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index f02831b085..70436007ec 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -34,16 +34,9 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
>
>  static struct strvec default_args = STRVEC_INIT;
>
> -/*
> - * TODO: convert this use of commit->object.flags to commit-slab
> - * instead to store a pointer to ref name directly. Then use the same
> - * UNINTERESTING definition from revision.h here.
> - */
>  #define UNINTERESTING	01

... it is a bit premature to lose this TODO comment (which was
written, inspired by what I wrote ages ago, in [*1*]), until that
happens.

On the other hand, you can and should lose our own #define
UNINTERSTING here even with this "slab stores a single u64 word"
rewrite, and instead use the common one from <revision.h> header
file.

Thanks.


[Reference]

*1* https://lore.kernel.org/git/xmqq36yud9bp.fsf@gitster-ct.c.googlers.com/