@matheusbsbs / matheus-bagliano / commits / 4844812b9e
Fork 0

commit-graph: introduce commit_graph_data_slab

The struct commit is used in many contexts. However, members `generation` and `graph_pos` are only used for commit-graph related operations and otherwise waste memory. This wastage would have been more pronounced as we transition to generation number v2, which uses 64-bit generation number instead of current 32-bits. As they are often accessed together, let's introduce struct commit_graph_data and move them to a commit_graph_data slab. While the overall test suite runs just as fast as master, (series: 26m48s, master: 27m34s, faster by 2.87%), certain commands like `git merge-base --is-ancestor` were slowed by 40% as discovered by Szeder Gábor [1]. After minimizing commit-slab access, the slow down persists but is closer to 20%. Derrick Stolee believes the slow down is attributable to the underlying algorithm rather than the slowness of commit-slab access [2] and we will follow-up in a later series. [1]: https://lore.kernel.org/git/20200607195347.GA8232@szeder.dev/ [2]: https://lore.kernel.org/git/13db757a-9412-7f1e-805c-8a028c4ab2b1@gmail.com/ 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 4844812b9ec9bc6ffdc2da2c54d9146ff4c97d94
2 files changed +78 -10
commit-graph.c
+68 -10
@@ -87,6 +87,58 @@ static int commit_pos_cmp(const void *va, const void *vb)
87 commit_pos_at(&commit_pos, b);
88 }
89
90 +define_commit_slab(commit_graph_data_slab, struct commit_graph_data);
91 +static struct commit_graph_data_slab commit_graph_data_slab =
92 + COMMIT_SLAB_INIT(1, commit_graph_data_slab);
93 +
94 +uint32_t commit_graph_position(const struct commit *c)
95 +{
96 + struct commit_graph_data *data =
97 + commit_graph_data_slab_peek(&commit_graph_data_slab, c);
98 +
99 + return data ? data->graph_pos : COMMIT_NOT_FROM_GRAPH;
100 +}
101 +
102 +uint32_t commit_graph_generation(const struct commit *c)
103 +{
104 + struct commit_graph_data *data =
105 + commit_graph_data_slab_peek(&commit_graph_data_slab, c);
106 +
107 + if (!data)
108 + return GENERATION_NUMBER_INFINITY;
109 + else if (data->graph_pos == COMMIT_NOT_FROM_GRAPH)
110 + return GENERATION_NUMBER_INFINITY;
111 +
112 + return data->generation;
113 +}
114 +
115 +static struct commit_graph_data *commit_graph_data_at(const struct commit *c)
116 +{
117 + unsigned int i, nth_slab;
118 + struct commit_graph_data *data =
119 + commit_graph_data_slab_peek(&commit_graph_data_slab, c);
120 +
121 + if (data)
122 + return data;
123 +
124 + nth_slab = c->index / commit_graph_data_slab.slab_size;
125 + data = commit_graph_data_slab_at(&commit_graph_data_slab, c);
126 +
127 + /*
128 + * commit-slab initializes elements with zero, overwrite this with
129 + * COMMIT_NOT_FROM_GRAPH for graph_pos.
130 + *
131 + * We avoid initializing generation with checking if graph position
132 + * is not COMMIT_NOT_FROM_GRAPH.
133 + */
134 + for (i = 0; i < commit_graph_data_slab.slab_size; i++) {
135 + commit_graph_data_slab.slab[nth_slab][i].graph_pos =
136 + COMMIT_NOT_FROM_GRAPH;
137 + }
138 +
139 + return data;
140 +}
141 +
142 static int commit_gen_cmp(const void *va, const void *vb)
143 {
144 const struct commit *a = *(const struct commit **)va;
@@ -1020,7 +1072,7 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,
1072 else
1073 packedDate[0] = 0;
1074
1023 - packedDate[0] |= htonl((*list)->generation << 2);
1075 + packedDate[0] |= htonl(commit_graph_data_at(*list)->generation << 2);
1076
1077 packedDate[1] = htonl((*list)->date);
1078 hashwrite(f, packedDate, 8);
@@ -1251,9 +1303,11 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
1303 _("Computing commit graph generation numbers"),
1304 ctx->commits.nr);
1305 for (i = 0; i < ctx->commits.nr; i++) {
1306 + uint32_t generation = commit_graph_data_at(ctx->commits.list[i])->generation;
1307 +
1308 display_progress(ctx->progress, i + 1);
1255 - if (ctx->commits.list[i]->generation != GENERATION_NUMBER_INFINITY &&
1256 - ctx->commits.list[i]->generation != GENERATION_NUMBER_ZERO)
1309 + if (generation != GENERATION_NUMBER_INFINITY &&
1310 + generation != GENERATION_NUMBER_ZERO)
1311 continue;
1312
1313 commit_list_insert(ctx->commits.list[i], &list);
@@ -1264,22 +1318,26 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
1318 uint32_t max_generation = 0;
1319
1320 for (parent = current->parents; parent; parent = parent->next) {
1267 - if (parent->item->generation == GENERATION_NUMBER_INFINITY ||
1268 - parent->item->generation == GENERATION_NUMBER_ZERO) {
1321 + generation = commit_graph_data_at(parent->item)->generation;
1322 +
1323 + if (generation == GENERATION_NUMBER_INFINITY ||
1324 + generation == GENERATION_NUMBER_ZERO) {
1325 all_parents_computed = 0;
1326 commit_list_insert(parent->item, &list);
1327 break;
1272 - } else if (parent->item->generation > max_generation) {
1273 - max_generation = parent->item->generation;
1328 + } else if (generation > max_generation) {
1329 + max_generation = generation;
1330 }
1331 }
1332
1333 if (all_parents_computed) {
1278 - current->generation = max_generation + 1;
1334 + struct commit_graph_data *data = commit_graph_data_at(current);
1335 +
1336 + data->generation = max_generation + 1;
1337 pop_commit(&list);
1338
1281 - if (current->generation > GENERATION_NUMBER_MAX)
1282 - current->generation = GENERATION_NUMBER_MAX;
1339 + if (data->generation > GENERATION_NUMBER_MAX)
1340 + data->generation = GENERATION_NUMBER_MAX;
1341 }
1342 }
1343 }
commit-graph.h
+10
@@ -135,4 +135,14 @@ void free_commit_graph(struct commit_graph *);
135 */
136 void disable_commit_graph(struct repository *r);
137
138 +struct commit_graph_data {
139 + uint32_t graph_pos;
140 + uint32_t generation;
141 +};
142 +
143 +/*
144 + * Commits should be parsed before accessing generation, graph positions.
145 + */
146 +uint32_t commit_graph_generation(const struct commit *);
147 +uint32_t commit_graph_position(const struct commit *);
148 #endif