Skip to content

Commit c752ad0

Browse files
abhishekkumar2718gitster
abhishekkumar2718
authored and
gitster
committed
commit-graph: minimize commit_graph_data_slab access
In an earlier patch, multiple struct acccesses to `graph_pos` and `generation` were auto-converted to multiple method calls. Since the values are fixed and commit-slab access costly, we would be better off with storing the values as a local variable and reusing it. Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent c49c82a commit c752ad0

File tree

5 files changed

+79
-43
lines changed

5 files changed

+79
-43
lines changed

‎bloom.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,16 @@ static int load_bloom_filter_from_graph(struct commit_graph *g,
3333
struct commit *c)
3434
{
3535
uint32_t lex_pos, start_index, end_index;
36+
uint32_t graph_pos = commit_graph_position(c);
3637

37-
while (commit_graph_position(c) < g->num_commits_in_base)
38+
while (graph_pos < g->num_commits_in_base)
3839
g = g->base_graph;
3940

4041
/* The commit graph commit 'c' lives in doesn't carry bloom filters. */
4142
if (!g->chunk_bloom_indexes)
4243
return 0;
4344

44-
lex_pos = commit_graph_position(c) - g->num_commits_in_base;
45+
lex_pos = graph_pos - g->num_commits_in_base;
4546

4647
end_index = get_be32(g->chunk_bloom_indexes + 4 * lex_pos);
4748

‎commit-graph.c‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,12 @@ static int commit_gen_cmp(const void *va, const void *vb)
144144
const struct commit *a = *(const struct commit **)va;
145145
const struct commit *b = *(const struct commit **)vb;
146146

147+
uint32_t generation_a = commit_graph_generation(a);
148+
uint32_t generation_b = commit_graph_generation(b);
147149
/* lower generation commits first */
148-
if (commit_graph_generation(a) < commit_graph_generation(b))
150+
if (generation_a < generation_b)
149151
return -1;
150-
else if (commit_graph_generation(a) > commit_graph_generation(b))
152+
else if (generation_a > generation_b)
151153
return 1;
152154

153155
/* use date as a heuristic when generations are equal */
@@ -729,15 +731,18 @@ static struct commit_list **insert_parent_or_die(struct repository *r,
729731
static void fill_commit_graph_info(struct commit *item, struct commit_graph *g, uint32_t pos)
730732
{
731733
const unsigned char *commit_data;
734+
struct commit_graph_data *graph_data;
732735
uint32_t lex_index;
733736

734737
while (pos < g->num_commits_in_base)
735738
g = g->base_graph;
736739

737740
lex_index = pos - g->num_commits_in_base;
738741
commit_data = g->chunk_commit_data + GRAPH_DATA_WIDTH * lex_index;
739-
commit_graph_data_at(item)->graph_pos = pos;
740-
commit_graph_data_at(item)->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
742+
743+
graph_data = commit_graph_data_at(item);
744+
graph_data->graph_pos = pos;
745+
graph_data->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
741746
}
742747

743748
static inline void set_commit_tree(struct commit *c, struct tree *t)
@@ -753,6 +758,7 @@ static int fill_commit_in_graph(struct repository *r,
753758
uint32_t *parent_data_ptr;
754759
uint64_t date_low, date_high;
755760
struct commit_list **pptr;
761+
struct commit_graph_data *graph_data;
756762
const unsigned char *commit_data;
757763
uint32_t lex_index;
758764

@@ -766,7 +772,8 @@ static int fill_commit_in_graph(struct repository *r,
766772
* Store the "full" position, but then use the
767773
* "local" position for the rest of the calculation.
768774
*/
769-
commit_graph_data_at(item)->graph_pos = pos;
775+
graph_data = commit_graph_data_at(item);
776+
graph_data->graph_pos = pos;
770777
lex_index = pos - g->num_commits_in_base;
771778

772779
commit_data = g->chunk_commit_data + (g->hash_len + 16) * lex_index;
@@ -779,7 +786,7 @@ static int fill_commit_in_graph(struct repository *r,
779786
date_low = get_be32(commit_data + g->hash_len + 12);
780787
item->date = (timestamp_t)((date_high << 32) | date_low);
781788

782-
commit_graph_data_at(item)->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
789+
graph_data->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
783790

784791
pptr = &item->parents;
785792

@@ -811,8 +818,9 @@ static int fill_commit_in_graph(struct repository *r,
811818

812819
static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uint32_t *pos)
813820
{
814-
if (commit_graph_position(item) != COMMIT_NOT_FROM_GRAPH) {
815-
*pos = commit_graph_position(item);
821+
uint32_t graph_pos = commit_graph_position(item);
822+
if (graph_pos != COMMIT_NOT_FROM_GRAPH) {
823+
*pos = graph_pos;
816824
return 1;
817825
} else {
818826
struct commit_graph *cur_g = g;
@@ -867,12 +875,13 @@ static struct tree *load_tree_for_commit(struct repository *r,
867875
{
868876
struct object_id oid;
869877
const unsigned char *commit_data;
878+
uint32_t graph_pos = commit_graph_position(c);
870879

871-
while (commit_graph_position(c) < g->num_commits_in_base)
880+
while (graph_pos < g->num_commits_in_base)
872881
g = g->base_graph;
873882

874883
commit_data = g->chunk_commit_data +
875-
GRAPH_DATA_WIDTH * (commit_graph_position(c) - g->num_commits_in_base);
884+
GRAPH_DATA_WIDTH * (graph_pos - g->num_commits_in_base);
876885

877886
hashcpy(oid.hash, commit_data);
878887
set_commit_tree(c, lookup_tree(r, &oid));
@@ -2299,6 +2308,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
22992308
struct commit *graph_commit, *odb_commit;
23002309
struct commit_list *graph_parents, *odb_parents;
23012310
uint32_t max_generation = 0;
2311+
uint32_t generation;
23022312

23032313
display_progress(progress, i + 1);
23042314
hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
@@ -2337,8 +2347,9 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
23372347
oid_to_hex(&graph_parents->item->object.oid),
23382348
oid_to_hex(&odb_parents->item->object.oid));
23392349

2340-
if (commit_graph_generation(graph_parents->item) > max_generation)
2341-
max_generation = commit_graph_generation(graph_parents->item);
2350+
generation = commit_graph_generation(graph_parents->item);
2351+
if (generation > max_generation)
2352+
max_generation = generation;
23422353

23432354
graph_parents = graph_parents->next;
23442355
odb_parents = odb_parents->next;
@@ -2368,10 +2379,11 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
23682379
if (max_generation == GENERATION_NUMBER_MAX)
23692380
max_generation--;
23702381

2371-
if (commit_graph_generation(graph_commit) != max_generation + 1)
2382+
generation = commit_graph_generation(graph_commit);
2383+
if (generation != max_generation + 1)
23722384
graph_report(_("commit-graph generation for commit %s is %u != %u"),
23732385
oid_to_hex(&cur_oid),
2374-
commit_graph_generation(graph_commit),
2386+
generation,
23752387
max_generation + 1);
23762388

23772389
if (graph_commit->date != odb_commit->date)

‎commit-reach.c‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,15 @@ static struct commit_list *paint_down_to_common(struct repository *r,
5858
struct commit *commit = prio_queue_get(&queue);
5959
struct commit_list *parents;
6060
int flags;
61+
uint32_t generation = commit_graph_generation(commit);
6162

62-
if (min_generation && commit_graph_generation(commit) > last_gen)
63+
if (min_generation && generation > last_gen)
6364
BUG("bad generation skip %8x > %8x at %s",
64-
commit_graph_generation(commit), last_gen,
65+
generation, last_gen,
6566
oid_to_hex(&commit->object.oid));
66-
last_gen = commit_graph_generation(commit);
67+
last_gen = generation;
6768

68-
if (commit_graph_generation(commit) < min_generation)
69+
if (generation < min_generation)
6970
break;
7071

7172
flags = commit->object.flags & (PARENT1 | PARENT2 | STALE);
@@ -181,13 +182,15 @@ static int remove_redundant(struct repository *r, struct commit **array, int cnt
181182
if (redundant[i])
182183
continue;
183184
for (j = filled = 0; j < cnt; j++) {
185+
uint32_t curr_generation;
184186
if (i == j || redundant[j])
185187
continue;
186188
filled_index[filled] = j;
187189
work[filled++] = array[j];
188190

189-
if (commit_graph_generation(array[j]) < min_generation)
190-
min_generation = commit_graph_generation(array[j]);
191+
curr_generation = commit_graph_generation(array[j]);
192+
if (curr_generation < min_generation)
193+
min_generation = curr_generation;
191194
}
192195
common = paint_down_to_common(r, array[i], filled,
193196
work, min_generation);
@@ -316,23 +319,26 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
316319
{
317320
struct commit_list *bases;
318321
int ret = 0, i;
319-
uint32_t min_generation = GENERATION_NUMBER_INFINITY;
322+
uint32_t generation, min_generation = GENERATION_NUMBER_INFINITY;
320323

321324
if (repo_parse_commit(r, commit))
322325
return ret;
323326
for (i = 0; i < nr_reference; i++) {
324327
if (repo_parse_commit(r, reference[i]))
325328
return ret;
326-
if (commit_graph_generation(reference[i]) < min_generation)
327-
min_generation = commit_graph_generation(reference[i]);
329+
330+
generation = commit_graph_generation(reference[i]);
331+
if (generation < min_generation)
332+
min_generation = generation;
328333
}
329334

330-
if (commit_graph_generation(commit) > min_generation)
335+
generation = commit_graph_generation(commit);
336+
if (generation > min_generation)
331337
return ret;
332338

333339
bases = paint_down_to_common(r, commit,
334340
nr_reference, reference,
335-
commit_graph_generation(commit));
341+
generation);
336342
if (commit->object.flags & PARENT2)
337343
ret = 1;
338344
clear_commit_marks(commit, all_flags);
@@ -490,10 +496,12 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
490496
const struct commit_list *p;
491497

492498
for (p = want; p; p = p->next) {
499+
uint32_t generation;
493500
struct commit *c = p->item;
494501
load_commit_graph_info(the_repository, c);
495-
if (commit_graph_generation(c) < cutoff)
496-
cutoff = commit_graph_generation(c);
502+
generation = commit_graph_generation(c);
503+
if (generation < cutoff)
504+
cutoff = generation;
497505
}
498506

499507
result = contains_test(candidate, want, cache, cutoff);
@@ -544,9 +552,12 @@ static int compare_commits_by_gen(const void *_a, const void *_b)
544552
const struct commit *a = *(const struct commit * const *)_a;
545553
const struct commit *b = *(const struct commit * const *)_b;
546554

547-
if (commit_graph_generation(a) < commit_graph_generation(b))
555+
uint32_t generation_a = commit_graph_generation(a);
556+
uint32_t generation_b = commit_graph_generation(b);
557+
558+
if (generation_a < generation_b)
548559
return -1;
549-
if (commit_graph_generation(a) > commit_graph_generation(b))
560+
if (generation_a > generation_b)
550561
return 1;
551562
return 0;
552563
}
@@ -662,23 +673,27 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to,
662673
add_object_array(&from_iter->item->object, NULL, &from_objs);
663674

664675
if (!parse_commit(from_iter->item)) {
676+
uint32_t generation;
665677
if (from_iter->item->date < min_commit_date)
666678
min_commit_date = from_iter->item->date;
667679

668-
if (commit_graph_generation(from_iter->item) < min_generation)
669-
min_generation = commit_graph_generation(from_iter->item);
680+
generation = commit_graph_generation(from_iter->item);
681+
if (generation < min_generation)
682+
min_generation = generation;
670683
}
671684

672685
from_iter = from_iter->next;
673686
}
674687

675688
while (to_iter) {
676689
if (!parse_commit(to_iter->item)) {
690+
uint32_t generation;
677691
if (to_iter->item->date < min_commit_date)
678692
min_commit_date = to_iter->item->date;
679693

680-
if (commit_graph_generation(to_iter->item) < min_generation)
681-
min_generation = commit_graph_generation(to_iter->item);
694+
generation = commit_graph_generation(to_iter->item);
695+
if (generation < min_generation)
696+
min_generation = generation;
682697
}
683698

684699
to_iter->item->object.flags |= PARENT2;
@@ -718,11 +733,13 @@ struct commit_list *get_reachable_subset(struct commit **from, int nr_from,
718733
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
719734

720735
for (item = to; item < to_last; item++) {
736+
uint32_t generation;
721737
struct commit *c = *item;
722738

723739
parse_commit(c);
724-
if (commit_graph_generation(c) < min_generation)
725-
min_generation = commit_graph_generation(c);
740+
generation = commit_graph_generation(c);
741+
if (generation < min_generation)
742+
min_generation = generation;
726743

727744
if (!(c->object.flags & PARENT1)) {
728745
c->object.flags |= PARENT1;

‎commit.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -729,11 +729,13 @@ int compare_commits_by_author_date(const void *a_, const void *b_,
729729
int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused)
730730
{
731731
const struct commit *a = a_, *b = b_;
732+
const uint32_t generation_a = commit_graph_generation(a),
733+
generation_b = commit_graph_generation(b);
732734

733735
/* newer commits first */
734-
if (commit_graph_generation(a) < commit_graph_generation(b))
736+
if (generation_a < generation_b)
735737
return 1;
736-
else if (commit_graph_generation(a) > commit_graph_generation(b))
738+
else if (generation_a > generation_b)
737739
return -1;
738740

739741
/* use date as a heuristic when generations are equal */

‎revision.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3413,15 +3413,17 @@ static void init_topo_walk(struct rev_info *revs)
34133413
info->min_generation = GENERATION_NUMBER_INFINITY;
34143414
for (list = revs->commits; list; list = list->next) {
34153415
struct commit *c = list->item;
3416+
uint32_t generation;
34163417

34173418
if (parse_commit_gently(c, 1))
34183419
continue;
34193420

34203421
test_flag_and_insert(&info->explore_queue, c, TOPO_WALK_EXPLORED);
34213422
test_flag_and_insert(&info->indegree_queue, c, TOPO_WALK_INDEGREE);
34223423

3423-
if (commit_graph_generation(c) < info->min_generation)
3424-
info->min_generation = commit_graph_generation(c);
3424+
generation = commit_graph_generation(c);
3425+
if (generation < info->min_generation)
3426+
info->min_generation = generation;
34253427

34263428
*(indegree_slab_at(&info->indegree, c)) = 1;
34273429

@@ -3472,15 +3474,17 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
34723474
for (p = commit->parents; p; p = p->next) {
34733475
struct commit *parent = p->item;
34743476
int *pi;
3477+
uint32_t generation;
34753478

34763479
if (parent->object.flags & UNINTERESTING)
34773480
continue;
34783481

34793482
if (parse_commit_gently(parent, 1) < 0)
34803483
continue;
34813484

3482-
if (commit_graph_generation(parent) < info->min_generation) {
3483-
info->min_generation = commit_graph_generation(parent);
3485+
generation = commit_graph_generation(parent);
3486+
if (generation < info->min_generation) {
3487+
info->min_generation = generation;
34843488
compute_indegrees_to_depth(revs, info->min_generation);
34853489
}
34863490

0 commit comments

Comments
 (0)