Skip to content

Commit 7d4da97

Browse files
committed
revision: introduce revision_walk_ops for get_revision_1()
Extract the per-mode dispatch logic in get_revision_1() into a revision_walk_ops struct with function pointers for next() and expand(). Each walk mode (reflog, topo, streaming, no_walk, limited) gets its own small helper functions, and get_walk_ops() selects the right ops table once per walk. This replaces the scattered if/else chains that checked the same mode conditions at multiple points in the loop body. The loop is now mode-agnostic: it calls ops->next() to pop the next commit and ops->expand() to process parents, with NULL expand for limited walks that skip parent processing entirely. No functional change. Signed-off-by: Kristofer Karlsson <krka@spotify.com>
1 parent 9e80c1a commit 7d4da97

2 files changed

Lines changed: 104 additions & 30 deletions

File tree

revision.c

Lines changed: 102 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3250,8 +3250,13 @@ static void free_void_commit_list(void *list)
32503250
commit_list_free(list);
32513251
}
32523252

3253+
static const struct revision_walk_ops *get_walk_ops(struct rev_info *revs);
3254+
32533255
void release_revisions(struct rev_info *revs)
32543256
{
3257+
if (revs->walk_ops && revs->walk_ops != get_walk_ops(revs))
3258+
BUG("walk_ops changed after initial selection");
3259+
32553260
commit_list_free(revs->commits);
32563261
clear_prio_queue(&revs->commit_queue);
32573262
commit_list_free(revs->ancestry_path_bottoms);
@@ -3942,12 +3947,27 @@ void rev_info_commit_list_to_queue(struct rev_info *revs)
39423947
}
39433948

39443949

3950+
/*
3951+
* Reset walk machinery so the same rev_info can be walked again.
3952+
* The bitmap code does this: it walks haves, then walks wants on the
3953+
* same rev_info. Without this reset, the cached walk_ops would skip
3954+
* re-initialization (e.g. draining the commit list into the priority
3955+
* queue), causing the second walk to read from an empty queue.
3956+
*/
3957+
static void reset_walk_state(struct rev_info *revs)
3958+
{
3959+
revs->walk_ops = NULL;
3960+
clear_prio_queue(&revs->commit_queue);
3961+
}
3962+
39453963
int prepare_revision_walk(struct rev_info *revs)
39463964
{
39473965
int i;
39483966
struct object_array old_pending;
39493967
struct commit_list **next = &revs->commits;
39503968

3969+
reset_walk_state(revs);
3970+
39513971
memcpy(&old_pending, &revs->pending, sizeof(old_pending));
39523972
revs->pending.nr = 0;
39533973
revs->pending.alloc = 0;
@@ -4322,46 +4342,98 @@ static void track_linear(struct rev_info *revs, struct commit *commit)
43224342
revs->previous_parents = commit_list_copy(commit->parents);
43234343
}
43244344

4345+
struct revision_walk_ops {
4346+
void (*init)(struct rev_info *);
4347+
struct commit *(*next)(struct rev_info *);
4348+
int (*expand)(struct rev_info *, struct commit *);
4349+
};
4350+
4351+
static struct commit *next_reflog(struct rev_info *revs)
4352+
{
4353+
struct commit *commit = next_reflog_entry(revs->reflog_info);
4354+
if (commit)
4355+
commit->object.flags &= ~(ADDED | SEEN | SHOWN);
4356+
return commit;
4357+
}
4358+
4359+
static int expand_reflog(struct rev_info *revs, struct commit *commit)
4360+
{
4361+
try_to_simplify_commit(revs, commit);
4362+
return 0;
4363+
}
4364+
4365+
static int expand_topo(struct rev_info *revs, struct commit *commit)
4366+
{
4367+
expand_topo_walk(revs, commit);
4368+
return 0;
4369+
}
4370+
4371+
static struct commit *next_streaming(struct rev_info *revs)
4372+
{
4373+
return prio_queue_get(&revs->commit_queue);
4374+
}
4375+
4376+
static int expand_streaming(struct rev_info *revs, struct commit *commit)
4377+
{
4378+
return process_parents(revs, commit, &revs->commit_queue);
4379+
}
4380+
4381+
static struct commit *next_commit_list(struct rev_info *revs)
4382+
{
4383+
return pop_commit(&revs->commits);
4384+
}
4385+
4386+
static int expand_no_walk(struct rev_info *revs, struct commit *commit)
4387+
{
4388+
return process_parents(revs, commit, NULL);
4389+
}
4390+
4391+
static struct revision_walk_ops reflog_ops =
4392+
{ NULL, next_reflog, expand_reflog };
4393+
static struct revision_walk_ops topo_ops =
4394+
{ NULL, next_topo_commit, expand_topo };
4395+
static struct revision_walk_ops streaming_ops =
4396+
{ rev_info_commit_list_to_queue, next_streaming, expand_streaming };
4397+
static struct revision_walk_ops no_walk_ops =
4398+
{ NULL, next_commit_list, expand_no_walk };
4399+
static struct revision_walk_ops limited_ops =
4400+
{ NULL, next_commit_list, NULL };
4401+
4402+
static const struct revision_walk_ops *get_walk_ops(struct rev_info *revs)
4403+
{
4404+
if (revs->reflog_info)
4405+
return &reflog_ops;
4406+
if (revs->topo_walk_info)
4407+
return &topo_ops;
4408+
if (revs->no_walk)
4409+
return &no_walk_ops;
4410+
if (!revs->limited)
4411+
return &streaming_ops;
4412+
return &limited_ops;
4413+
}
4414+
43254415
static struct commit *get_revision_1(struct rev_info *revs)
43264416
{
4417+
const struct revision_walk_ops *ops;
4418+
4419+
if (!revs->walk_ops) {
4420+
revs->walk_ops = get_walk_ops(revs);
4421+
if (revs->walk_ops->init)
4422+
revs->walk_ops->init(revs);
4423+
}
4424+
ops = revs->walk_ops;
4425+
43274426
while (1) {
4328-
struct commit *commit;
4329-
4330-
if (revs->reflog_info)
4331-
commit = next_reflog_entry(revs->reflog_info);
4332-
else if (revs->topo_walk_info)
4333-
commit = next_topo_commit(revs);
4334-
else if (revs->limited || revs->no_walk)
4335-
commit = pop_commit(&revs->commits);
4336-
else {
4337-
if (!revs->commit_queue.nr && revs->commits)
4338-
rev_info_commit_list_to_queue(revs);
4339-
commit = prio_queue_get(&revs->commit_queue);
4340-
}
4427+
struct commit *commit = ops->next(revs);
43414428

43424429
if (!commit)
43434430
return NULL;
43444431

4345-
if (revs->reflog_info)
4346-
commit->object.flags &= ~(ADDED | SEEN | SHOWN);
4347-
4348-
/*
4349-
* If we haven't done the list limiting, we need to look at
4350-
* the parents here. We also need to do the date-based limiting
4351-
* that we'd otherwise have done in limit_list().
4352-
*/
4353-
if (!revs->limited) {
4432+
if (ops->expand) {
43544433
if (revs->max_age != -1 &&
43554434
comparison_date(revs, commit) < revs->max_age)
43564435
continue;
4357-
4358-
if (revs->reflog_info)
4359-
try_to_simplify_commit(revs, commit);
4360-
else if (revs->topo_walk_info)
4361-
expand_topo_walk(revs, commit);
4362-
else if (process_parents(revs, commit,
4363-
revs->no_walk ?
4364-
NULL : &revs->commit_queue) < 0) {
4436+
if (ops->expand(revs, commit) < 0) {
43654437
if (!revs->ignore_missing_links)
43664438
die("Failed to traverse parents of commit %s",
43674439
oid_to_hex(&commit->object.oid));

revision.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ struct ref_exclusions {
120120
}
121121

122122
struct oidset;
123+
struct revision_walk_ops;
123124
struct topo_walk_info;
124125

125126
struct rev_info {
@@ -367,6 +368,7 @@ struct rev_info {
367368
struct revision_sources *sources;
368369

369370
struct topo_walk_info *topo_walk_info;
371+
const struct revision_walk_ops *walk_ops;
370372

371373
/* Commit graph bloom filter fields */
372374
/* The bloom filter key(s) for the pathspec */

0 commit comments

Comments
 (0)