@matheusbsbs / matheus-bagliano / commits / 312cff5207

Fork 0

bloom: split 'get_bloom_filter()' in two

'get_bloom_filter' takes a flag to control whether it will compute a Bloom filter if the requested one is missing. In the next patch, we'll add yet another parameter to this method, which would force all but one caller to specify an extra 'NULL' parameter at the end. Instead of doing this, split 'get_bloom_filter' into two functions: 'get_bloom_filter' and 'get_or_compute_bloom_filter'. The former only looks up a Bloom filter (and does not compute one if it's missing, thus dropping the 'compute_if_not_present' flag). The latter does compute missing Bloom filters, with an additional parameter to store whether or not it needed to do so. This simplifies many call-sites, since the majority of existing callers to 'get_bloom_filter' do not want missing Bloom filters to be computed (so they can drop the parameter entirely and use the simpler version of the function). While we're at it, instrument the new 'get_or_compute_bloom_filter()' with counters in the 'write_commit_graph_context' struct which store the number of filters that we did and didn't compute, as well as filters that were truncated. It would be nice to drop the 'compute_if_not_present' flag entirely, since all remaining callers of 'get_or_compute_bloom_filter' pass it as '1', but this will change in a future patch and hence cannot be removed. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Sep 16, 2020 at 14:07 UTC 312cff520742c933bde070be18c51c27e132cff1

7 files changed +62 -13

blame.c

+1 -1

@@ -1275,7 +1275,7 @@ static int maybe_changed_path(struct repository *r,

1275 if (commit_graph_generation(origin->commit) == GENERATION_NUMBER_INFINITY)

1276 return 1;

1277

1278 - filter = get_bloom_filter(r, origin->commit, 0);

1278 + filter = get_bloom_filter(r, origin->commit);

1279

1280 if (!filter)

1281 return 1;

bloom.c

+13 -3

@@ -177,9 +177,10 @@ static int pathmap_cmp(const void *hashmap_cmp_fn_data,

177 return strcmp(e1->path, e2->path);

178 }

179

180 -struct bloom_filter *get_bloom_filter(struct repository *r,

181 - struct commit *c,

182 - int compute_if_not_present)

180 +struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,

181 + struct commit *c,

182 + int compute_if_not_present,

183 + enum bloom_filter_computed *computed)

184 {

185 struct bloom_filter *filter;

186 struct bloom_filter_settings settings = DEFAULT_BLOOM_FILTER_SETTINGS;

@@ -187,6 +188,9 @@ struct bloom_filter *get_bloom_filter(struct repository *r,

188 struct diff_options diffopt;

189 int max_changes = 512;

190

191 + if (computed)

192 + *computed = BLOOM_NOT_COMPUTED;

193 +

194 if (!bloom_filters.slab_size)

195 return NULL;

196

@@ -271,8 +275,14 @@ struct bloom_filter *get_bloom_filter(struct repository *r,

275 diff_free_filepair(diff_queued_diff.queue[i]);

276 filter->data = NULL;

277 filter->len = 0;

278 +

279 + if (computed)

280 + *computed |= BLOOM_TRUNC_LARGE;

281 }

282

283 + if (computed)

284 + *computed |= BLOOM_COMPUTED;

285 +

286 free(diff_queued_diff.queue);

287 DIFF_QUEUE_CLEAR(&diff_queued_diff);

288

bloom.h

+13 -3

@@ -89,9 +89,19 @@ void add_key_to_filter(const struct bloom_key *key,

90 void init_bloom_filters(void);

92 -struct bloom_filter *get_bloom_filter(struct repository *r,

93 - struct commit *c,

94 - int compute_if_not_present);

92 +enum bloom_filter_computed {

93 + BLOOM_NOT_COMPUTED = (1 << 0),

94 + BLOOM_COMPUTED = (1 << 1),

95 + BLOOM_TRUNC_LARGE = (1 << 2),

96 +};

97 +

98 +struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,

99 + struct commit *c,

100 + int compute_if_not_present,

101 + enum bloom_filter_computed *computed);

102 +

103 +#define get_bloom_filter(r, c) get_or_compute_bloom_filter( \

104 + (r), (c), 0, NULL)

105

106 int bloom_filter_contains(const struct bloom_filter *filter,

107 const struct bloom_key *key,

commit-graph.c

+31 -3

@@ -965,6 +965,10 @@ struct write_commit_graph_context {

965 const struct split_commit_graph_opts *split_opts;

966 size_t total_bloom_filter_data_size;

967 const struct bloom_filter_settings *bloom_settings;

968 +

969 + int count_bloom_filter_computed;

970 + int count_bloom_filter_not_computed;

971 + int count_bloom_filter_trunc_large;

972 };

973

974 static int write_graph_chunk_fanout(struct hashfile *f,

@@ -1176,7 +1180,7 @@ static int write_graph_chunk_bloom_indexes(struct hashfile *f,

1180 uint32_t cur_pos = 0;

1181

1182 while (list < last) {

1179 - struct bloom_filter *filter = get_bloom_filter(ctx->r, *list, 0);

1183 + struct bloom_filter *filter = get_bloom_filter(ctx->r, *list);

1184 size_t len = filter ? filter->len : 0;

1185 cur_pos += len;

1186 display_progress(ctx->progress, ++ctx->progress_cnt);

@@ -1216,7 +1220,7 @@ static int write_graph_chunk_bloom_data(struct hashfile *f,

1220 hashwrite_be32(f, ctx->bloom_settings->bits_per_entry);

1221

1222 while (list < last) {

1219 - struct bloom_filter *filter = get_bloom_filter(ctx->r, *list, 0);

1223 + struct bloom_filter *filter = get_bloom_filter(ctx->r, *list);

1224 size_t len = filter ? filter->len : 0;

1225

1226 display_progress(ctx->progress, ++ctx->progress_cnt);

@@ -1386,6 +1390,16 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)

1390 stop_progress(&ctx->progress);

1391 }

1392

1393 +static void trace2_bloom_filter_write_statistics(struct write_commit_graph_context *ctx)

1394 +{

1395 + trace2_data_intmax("commit-graph", ctx->r, "filter-computed",

1396 + ctx->count_bloom_filter_computed);

1397 + trace2_data_intmax("commit-graph", ctx->r, "filter-not-computed",

1398 + ctx->count_bloom_filter_not_computed);

1399 + trace2_data_intmax("commit-graph", ctx->r, "filter-trunc-large",

1400 + ctx->count_bloom_filter_trunc_large);

1401 +}

1402 +

1403 static void compute_bloom_filters(struct write_commit_graph_context *ctx)

1404 {

1405 int i;

@@ -1408,12 +1422,26 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)

1422 QSORT(sorted_commits, ctx->commits.nr, commit_gen_cmp);

1423

1424 for (i = 0; i < ctx->commits.nr; i++) {

1425 + enum bloom_filter_computed computed = 0;

1426 struct commit *c = sorted_commits[i];

1412 - struct bloom_filter *filter = get_bloom_filter(ctx->r, c, 1);

1427 + struct bloom_filter *filter = get_or_compute_bloom_filter(

1428 + ctx->r,

1429 + c,

1430 + 1,

1431 + &computed);

1432 + if (computed & BLOOM_COMPUTED) {

1433 + ctx->count_bloom_filter_computed++;

1434 + if (computed & BLOOM_TRUNC_LARGE)

1435 + ctx->count_bloom_filter_trunc_large++;

1436 + } else if (computed & BLOOM_NOT_COMPUTED)

1437 + ctx->count_bloom_filter_not_computed++;

1438 ctx->total_bloom_filter_data_size += sizeof(unsigned char) * filter->len;

1439 display_progress(progress, i + 1);

1440 }

1441

1442 + if (trace2_is_enabled())

1443 + trace2_bloom_filter_write_statistics(ctx);

1444 +

1445 free(sorted_commits);

1446 stop_progress(&progress);

1447 }

line-log.c

+1 -1

@@ -1159,7 +1159,7 @@ static int bloom_filter_check(struct rev_info *rev,

1159 return 1;

1160

1161 if (!rev->bloom_filter_settings ||

1162 - !(filter = get_bloom_filter(rev->repo, commit, 0)))

1162 + !(filter = get_bloom_filter(rev->repo, commit)))

1163 return 1;

1164

1165 if (!range)

revision.c

+1 -1

@@ -753,7 +753,7 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs,

753 if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY)

754 return -1;

755

756 - filter = get_bloom_filter(revs->repo, commit, 0);

756 + filter = get_bloom_filter(revs->repo, commit);

757

758 if (!filter) {

759 count_bloom_filter_not_present++;

t/helper/test-bloom.c

+2 -1

@@ -39,7 +39,8 @@ static void get_bloom_filter_for_commit(const struct object_id *commit_oid)

39 struct bloom_filter *filter;

40 setup_git_directory();

41 c = lookup_commit(the_repository, commit_oid);

42 - filter = get_bloom_filter(the_repository, c, 1);

42 + filter = get_or_compute_bloom_filter(the_repository, c, 1,

43 + NULL);

44 print_bloom_filter(filter);

45 }