bloom: introduce `deinit_bloom_filters()`

After we are done using Bloom filters, we do not currently clean up any memory allocated by the commit slab used to store those filters in the first place. Besides the bloom_filter structures themselves, there is mostly nothing to free() in the first place, since in the read-only path all Bloom filter's `data` members point to a memory mapped region in the commit-graph file itself. But when generating Bloom filters from scratch (or initializing truncated filters) we allocate additional memory to store the filter's data. Keep track of when we need to free() this additional chunk of memory by using an extra pointer `to_free`. Most of the time this will be NULL (indicating that we are representing an existing Bloom filter stored in a memory mapped region). When it is non-NULL, free it before discarding the Bloom filters slab. Suggested-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Jun 25, 2024 at 13:40 UTC 9c8a9ec787149dc4f4b278d9bd8ad94c96691a5f
3 files changed +22 -1
bloom.c
+15 -1
@@ -92,6 +92,7 @@ int load_bloom_filter_from_graph(struct commit_graph *g,
92 sizeof(unsigned char) * start_index +
93 BLOOMDATA_CHUNK_HEADER_SIZE);
94 filter->version = g->bloom_filter_settings->hash_version;
95 + filter->to_free = NULL;
96
97 return 1;
98 }
@@ -264,6 +265,18 @@ void init_bloom_filters(void)
265 init_bloom_filter_slab(&bloom_filters);
266 }
267
268 +static void free_one_bloom_filter(struct bloom_filter *filter)
269 +{
270 + if (!filter)
271 + return;
272 + free(filter->to_free);
273 +}
274 +
275 +void deinit_bloom_filters(void)
276 +{
277 + deep_clear_bloom_filter_slab(&bloom_filters, free_one_bloom_filter);
278 +}
279 +
280 static int pathmap_cmp(const void *hashmap_cmp_fn_data UNUSED,
281 const struct hashmap_entry *eptr,
282 const struct hashmap_entry *entry_or_key,
@@ -280,7 +293,7 @@ static int pathmap_cmp(const void *hashmap_cmp_fn_data UNUSED,
293 static void init_truncated_large_filter(struct bloom_filter *filter,
294 int version)
295 {
283 - filter->data = xmalloc(1);
296 + filter->data = filter->to_free = xmalloc(1);
297 filter->data[0] = 0xFF;
298 filter->len = 1;
299 filter->version = version;
@@ -482,6 +495,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
495 filter->len = 1;
496 }
497 CALLOC_ARRAY(filter->data, filter->len);
498 + filter->to_free = filter->data;
499
500 hashmap_for_each_entry(&pathmap, &iter, e, entry) {
501 struct bloom_key key;
bloom.h
+3
@@ -56,6 +56,8 @@ struct bloom_filter {
56 unsigned char *data;
57 size_t len;
58 int version;
59 +
60 + void *to_free;
61 };
62
63 /*
@@ -96,6 +98,7 @@ void add_key_to_filter(const struct bloom_key *key,
98 const struct bloom_filter_settings *settings);
99
100 void init_bloom_filters(void);
101 +void deinit_bloom_filters(void);
102
103 enum bloom_filter_computed {
104 BLOOM_NOT_COMPUTED = (1 << 0),
commit-graph.c
+4
@@ -828,6 +828,7 @@ struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)
828 void close_commit_graph(struct raw_object_store *o)
829 {
830 clear_commit_graph_data_slab(&commit_graph_data_slab);
831 + deinit_bloom_filters();
832 free_commit_graph(o->commit_graph);
833 o->commit_graph = NULL;
834 }
@@ -2646,6 +2647,9 @@ int write_commit_graph(struct object_directory *odb,
2647
2648 res = write_commit_graph_file(ctx);
2649
2650 + if (ctx->changed_paths)
2651 + deinit_bloom_filters();
2652 +
2653 if (ctx->split)
2654 mark_commit_graphs(ctx);
2655