* [PATCH] show-branch: convert object.flags usage to a commit-slab
@ 2026-07-14 18:30 Gatla Vishweshwar Reddy
2026-07-14 20:01 ` [PATCH v2] show-branch: convert object.flags to commit-slab with uint64_t Gatla Vishweshwar Reddy
0 siblings, 1 reply; 3+ messages in thread
From: Gatla Vishweshwar Reddy @ 2026-07-14 18:30 UTC (permalink / raw)
To: git; +Cc: 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2] show-branch: convert object.flags to commit-slab with uint64_t
2026-07-14 18:30 [PATCH] show-branch: convert object.flags usage to a commit-slab Gatla Vishweshwar Reddy
@ 2026-07-14 20:01 ` Gatla Vishweshwar Reddy
2026-07-14 20:41 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Gatla Vishweshwar Reddy @ 2026-07-14 20:01 UTC (permalink / raw)
To: git; +Cc: 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] show-branch: convert object.flags to commit-slab with uint64_t
2026-07-14 20:01 ` [PATCH v2] show-branch: convert object.flags to commit-slab with uint64_t Gatla Vishweshwar Reddy
@ 2026-07-14 20:41 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2026-07-14 20:41 UTC (permalink / raw)
To: Gatla Vishweshwar Reddy; +Cc: git
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)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-14 20:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 18:30 [PATCH] show-branch: convert object.flags usage to a commit-slab Gatla Vishweshwar Reddy
2026-07-14 20:01 ` [PATCH v2] show-branch: convert object.flags to commit-slab with uint64_t Gatla Vishweshwar Reddy
2026-07-14 20:41 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox