@matheusbsbs / matheus-bagliano / commits / c752ad09c4
Fork 0

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>

Abhishek Kumar committed Jun 17, 2020 at 14:44 UTC c752ad09c4ea479e8d54d08637cc0e5709723208
5 files changed +79 -43
bloom.c
+3 -2
@@ -33,15 +33,16 @@ static int load_bloom_filter_from_graph(struct commit_graph *g,
33 struct commit *c)
34 {
35 uint32_t lex_pos, start_index, end_index;
36 + uint32_t graph_pos = commit_graph_position(c);
37
37 - while (commit_graph_position(c) < g->num_commits_in_base)
38 + while (graph_pos < g->num_commits_in_base)
39 g = g->base_graph;
40
41 /* The commit graph commit 'c' lives in doesn't carry bloom filters. */
42 if (!g->chunk_bloom_indexes)
43 return 0;
44
44 - lex_pos = commit_graph_position(c) - g->num_commits_in_base;
45 + lex_pos = graph_pos - g->num_commits_in_base;
46
47 end_index = get_be32(g->chunk_bloom_indexes + 4 * lex_pos);
48
commit-graph.c
+26 -14
@@ -144,10 +144,12 @@ static int commit_gen_cmp(const void *va, const void *vb)
144 const struct commit *a = *(const struct commit **)va;
145 const struct commit *b = *(const struct commit **)vb;
146
147 + uint32_t generation_a = commit_graph_generation(a);
148 + uint32_t generation_b = commit_graph_generation(b);
149 /* lower generation commits first */
148 - if (commit_graph_generation(a) < commit_graph_generation(b))
150 + if (generation_a < generation_b)
151 return -1;
150 - else if (commit_graph_generation(a) > commit_graph_generation(b))
152 + else if (generation_a > generation_b)
153 return 1;
154
155 /* use date as a heuristic when generations are equal */
@@ -729,6 +731,7 @@ static struct commit_list **insert_parent_or_die(struct repository *r,
731 static void fill_commit_graph_info(struct commit *item, struct commit_graph *g, uint32_t pos)
732 {
733 const unsigned char *commit_data;
734 + struct commit_graph_data *graph_data;
735 uint32_t lex_index;
736
737 while (pos < g->num_commits_in_base)
@@ -736,8 +739,10 @@ static void fill_commit_graph_info(struct commit *item, struct commit_graph *g,
739
740 lex_index = pos - g->num_commits_in_base;
741 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;
746 }
747
748 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,
758 uint32_t *parent_data_ptr;
759 uint64_t date_low, date_high;
760 struct commit_list **pptr;
761 + struct commit_graph_data *graph_data;
762 const unsigned char *commit_data;
763 uint32_t lex_index;
764
@@ -766,7 +772,8 @@ static int fill_commit_in_graph(struct repository *r,
772 * Store the "full" position, but then use the
773 * "local" position for the rest of the calculation.
774 */
769 - commit_graph_data_at(item)->graph_pos = pos;
775 + graph_data = commit_graph_data_at(item);
776 + graph_data->graph_pos = pos;
777 lex_index = pos - g->num_commits_in_base;
778
779 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,
786 date_low = get_be32(commit_data + g->hash_len + 12);
787 item->date = (timestamp_t)((date_high << 32) | date_low);
788
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;
790
791 pptr = &item->parents;
792
@@ -811,8 +818,9 @@ static int fill_commit_in_graph(struct repository *r,
818
819 static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uint32_t *pos)
820 {
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;
824 return 1;
825 } else {
826 struct commit_graph *cur_g = g;
@@ -867,12 +875,13 @@ static struct tree *load_tree_for_commit(struct repository *r,
875 {
876 struct object_id oid;
877 const unsigned char *commit_data;
878 + uint32_t graph_pos = commit_graph_position(c);
879
871 - while (commit_graph_position(c) < g->num_commits_in_base)
880 + while (graph_pos < g->num_commits_in_base)
881 g = g->base_graph;
882
883 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);
885
886 hashcpy(oid.hash, commit_data);
887 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)
2308 struct commit *graph_commit, *odb_commit;
2309 struct commit_list *graph_parents, *odb_parents;
2310 uint32_t max_generation = 0;
2311 + uint32_t generation;
2312
2313 display_progress(progress, i + 1);
2314 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)
2347 oid_to_hex(&graph_parents->item->object.oid),
2348 oid_to_hex(&odb_parents->item->object.oid));
2349
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;
2353
2354 graph_parents = graph_parents->next;
2355 odb_parents = odb_parents->next;
@@ -2368,10 +2379,11 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
2379 if (max_generation == GENERATION_NUMBER_MAX)
2380 max_generation--;
2381
2371 - if (commit_graph_generation(graph_commit) != max_generation + 1)
2382 + generation = commit_graph_generation(graph_commit);
2383 + if (generation != max_generation + 1)
2384 graph_report(_("commit-graph generation for commit %s is %u != %u"),
2385 oid_to_hex(&cur_oid),
2374 - commit_graph_generation(graph_commit),
2386 + generation,
2387 max_generation + 1);
2388
2389 if (graph_commit->date != odb_commit->date)
commit-reach.c
+38 -21
@@ -58,14 +58,15 @@ static struct commit_list *paint_down_to_common(struct repository *r,
58 struct commit *commit = prio_queue_get(&queue);
59 struct commit_list *parents;
60 int flags;
61 + uint32_t generation = commit_graph_generation(commit);
62
62 - if (min_generation && commit_graph_generation(commit) > last_gen)
63 + if (min_generation && generation > last_gen)
64 BUG("bad generation skip %8x > %8x at %s",
64 - commit_graph_generation(commit), last_gen,
65 + generation, last_gen,
66 oid_to_hex(&commit->object.oid));
66 - last_gen = commit_graph_generation(commit);
67 + last_gen = generation;
68
68 - if (commit_graph_generation(commit) < min_generation)
69 + if (generation < min_generation)
70 break;
71
72 flags = commit->object.flags & (PARENT1 | PARENT2 | STALE);
@@ -181,13 +182,15 @@ static int remove_redundant(struct repository *r, struct commit **array, int cnt
182 if (redundant[i])
183 continue;
184 for (j = filled = 0; j < cnt; j++) {
185 + uint32_t curr_generation;
186 if (i == j || redundant[j])
187 continue;
188 filled_index[filled] = j;
189 work[filled++] = array[j];
190
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;
194 }
195 common = paint_down_to_common(r, array[i], filled,
196 work, min_generation);
@@ -316,23 +319,26 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
319 {
320 struct commit_list *bases;
321 int ret = 0, i;
319 - uint32_t min_generation = GENERATION_NUMBER_INFINITY;
322 + uint32_t generation, min_generation = GENERATION_NUMBER_INFINITY;
323
324 if (repo_parse_commit(r, commit))
325 return ret;
326 for (i = 0; i < nr_reference; i++) {
327 if (repo_parse_commit(r, reference[i]))
328 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;
333 }
334
330 - if (commit_graph_generation(commit) > min_generation)
335 + generation = commit_graph_generation(commit);
336 + if (generation > min_generation)
337 return ret;
338
339 bases = paint_down_to_common(r, commit,
340 nr_reference, reference,
335 - commit_graph_generation(commit));
341 + generation);
342 if (commit->object.flags & PARENT2)
343 ret = 1;
344 clear_commit_marks(commit, all_flags);
@@ -490,10 +496,12 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
496 const struct commit_list *p;
497
498 for (p = want; p; p = p->next) {
499 + uint32_t generation;
500 struct commit *c = p->item;
501 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;
505 }
506
507 result = contains_test(candidate, want, cache, cutoff);
@@ -544,9 +552,12 @@ static int compare_commits_by_gen(const void *_a, const void *_b)
552 const struct commit *a = *(const struct commit * const *)_a;
553 const struct commit *b = *(const struct commit * const *)_b;
554
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)
559 return -1;
549 - if (commit_graph_generation(a) > commit_graph_generation(b))
560 + if (generation_a > generation_b)
561 return 1;
562 return 0;
563 }
@@ -662,11 +673,13 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to,
673 add_object_array(&from_iter->item->object, NULL, &from_objs);
674
675 if (!parse_commit(from_iter->item)) {
676 + uint32_t generation;
677 if (from_iter->item->date < min_commit_date)
678 min_commit_date = from_iter->item->date;
679
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;
683 }
684
685 from_iter = from_iter->next;
@@ -674,11 +687,13 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to,
687
688 while (to_iter) {
689 if (!parse_commit(to_iter->item)) {
690 + uint32_t generation;
691 if (to_iter->item->date < min_commit_date)
692 min_commit_date = to_iter->item->date;
693
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;
697 }
698
699 to_iter->item->object.flags |= PARENT2;
@@ -718,11 +733,13 @@ struct commit_list *get_reachable_subset(struct commit **from, int nr_from,
733 struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
734
735 for (item = to; item < to_last; item++) {
736 + uint32_t generation;
737 struct commit *c = *item;
738
739 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;
743
744 if (!(c->object.flags & PARENT1)) {
745 c->object.flags |= PARENT1;
commit.c
+4 -2
@@ -729,11 +729,13 @@ int compare_commits_by_author_date(const void *a_, const void *b_,
729 int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused)
730 {
731 const struct commit *a = a_, *b = b_;
732 + const uint32_t generation_a = commit_graph_generation(a),
733 + generation_b = commit_graph_generation(b);
734
735 /* newer commits first */
734 - if (commit_graph_generation(a) < commit_graph_generation(b))
736 + if (generation_a < generation_b)
737 return 1;
736 - else if (commit_graph_generation(a) > commit_graph_generation(b))
738 + else if (generation_a > generation_b)
739 return -1;
740
741 /* use date as a heuristic when generations are equal */
revision.c
+8 -4
@@ -3413,6 +3413,7 @@ static void init_topo_walk(struct rev_info *revs)
3413 info->min_generation = GENERATION_NUMBER_INFINITY;
3414 for (list = revs->commits; list; list = list->next) {
3415 struct commit *c = list->item;
3416 + uint32_t generation;
3417
3418 if (parse_commit_gently(c, 1))
3419 continue;
@@ -3420,8 +3421,9 @@ static void init_topo_walk(struct rev_info *revs)
3421 test_flag_and_insert(&info->explore_queue, c, TOPO_WALK_EXPLORED);
3422 test_flag_and_insert(&info->indegree_queue, c, TOPO_WALK_INDEGREE);
3423
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;
3427
3428 *(indegree_slab_at(&info->indegree, c)) = 1;
3429
@@ -3472,6 +3474,7 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
3474 for (p = commit->parents; p; p = p->next) {
3475 struct commit *parent = p->item;
3476 int *pi;
3477 + uint32_t generation;
3478
3479 if (parent->object.flags & UNINTERESTING)
3480 continue;
@@ -3479,8 +3482,9 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
3482 if (parse_commit_gently(parent, 1) < 0)
3483 continue;
3484
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;
3488 compute_indegrees_to_depth(revs, info->min_generation);
3489 }
3490