[PATCH v2] delta-islands: free island_marks and bitmaps
- From
- Eric Wong <e@80x24.org>
- Date
- Feb 2, 2023, 09:42 UTC
- Message-ID
- <20230202094217.M955476@dcvr>
- In-Reply-To
- <230202.86mt5wq1i7.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 6 quoted lines
> On Thu, Feb 02 2023, Eric Wong wrote:
>
> > + kh_foreach_value(island_marks, bitmap, {
> > + if (--bitmap->refcount == 0)
>
> Style nit: if (!--x) rather than if (--x == 0) ?Oops, fixed in v2
Show 7 quoted lines
> > + free(bitmap); > > + }); > > + kh_destroy_oid_map(island_marks); > > + island_marks = (void *)1; /* crash on unintended future use */ > > This seems counter-productive. If you just leave it free'd then various > analysis tools will spot a use-after-free earlier, won't they?
*shrug* I thought it might be better to use an explicitly bad pointer to catch lifetime issues that I might've missed from reading the code. Since I've run and tested at this point, it probably doesn't matter, now. So I'll just omit the assignment and save some icache footprint.
Thanks for the review!
--------8<------- Subject: [PATCH] delta-islands: free island_marks and bitmaps
On my mirror of linux.git forkgroup with 780 islands, this saves nearly 4G of heap memory in pack-objects. This savings only benefits delta island users of pack bitmaps, as the process would otherwise be exiting anyways.
However, there's probably not many delta island users, but the majority of delta island users would also be pack bitmaps users.
Signed-off-by: Eric Wong <e@80x24.org> --- delta-islands.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/delta-islands.c b/delta-islands.c index 90c0d6958f..c09dab31a4 100644 --- a/delta-islands.c +++ b/delta-islands.c @@ -513,12 +513,28 @@ void propagate_island_marks(struct commit *commit) } } +static void free_island_marks(void) +{ + struct island_bitmap *bitmap; + + kh_foreach_value(island_marks, bitmap, { + if (!--bitmap->refcount) + free(bitmap); + }); + kh_destroy_oid_map(island_marks); +} + int compute_pack_layers(struct packing_data *to_pack) { uint32_t i; - if (!core_island_name || !island_marks) + if (!island_marks) + return 1; + + if (!core_island_name) { + free_island_marks(); return 1; + } for (i = 0; i < to_pack->nr_objects; ++i) { struct object_entry *entry = &to_pack->objects[i]; @@ -533,6 +549,7 @@ int compute_pack_layers(struct packing_data *to_pack) oe_set_layer(to_pack, entry, 0); } } + free_island_marks(); return 2; }