In 37dc6d8104 (builtin/repack.c: implement support for
`--max-cruft-size`, 2023-10-02), 'git repack' built on support for
multiple cruft packs in Git by instructing 'git pack-objects --cruft'
how to aggregate smaller cruft packs up to the provided threshold.
The implementation in 37dc6d8104 worked something like the following
pseudo-code:
total_size = 0;
for (p in cruft packs) {
if (p->pack_size + total_size < max_size) {
total_size += p->pack_size;
collapse(p)
} else {
retain(p);
}
}
The original idea behind this approach was that smaller cruft packs
would get combined together until the sum of their sizes was no larger
than the given max pack size.
There is a much simpler way to achieve this, however, which is to simply
combine *all* cruft packs which are smaller than the threshold,
regardless of what their sum is. With '--max-pack-size', 'pack-objects'
will split out the resulting pack into individual pack(s) if necessary
to ensure that the written pack(s) are each no larger than the provided
threshold.
This yields a slight behavior change, which is reflected in the removed
test. Previous to this change, we would aggregate smaller cruft packs
first, whereas now we will opportunistically combine as many cruft packs
as possible. As as result, that test is no longer relevant, and can be
deleted.
Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
---
builtin/repack.c | 38 ++-----------------------------------
t/t7704-repack-cruft.sh | 42 -----------------------------------------
2 files changed, 2 insertions(+), 78 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 75e3752353a..4d83d40f39f 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -1022,29 +1022,13 @@ static int write_filtered_pack(const struct pack_objects_args *args,
return finish_pack_objects_cmd(&cmd, names, local);
}
-static int existing_cruft_pack_cmp(const void *va, const void *vb)
-{
- struct packed_git *a = *(struct packed_git **)va;
- struct packed_git *b = *(struct packed_git **)vb;
-
- if (a->pack_size < b->pack_size)
- return -1;
- if (a->pack_size > b->pack_size)
- return 1;
- return 0;
-}
-
static void collapse_small_cruft_packs(FILE *in, size_t max_size,
struct existing_packs *existing)
{
- struct packed_git **existing_cruft, *p;
+ struct packed_git *p;
struct strbuf buf = STRBUF_INIT;
- size_t total_size = 0;
- size_t existing_cruft_nr = 0;
size_t i;
- ALLOC_ARRAY(existing_cruft, existing->cruft_packs.nr);
-
for (p = get_all_packs(the_repository); p; p = p->next) {
if (!(p->is_cruft && p->pack_local))
continue;
@@ -1056,24 +1040,7 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size,
if (!string_list_has_string(&existing->cruft_packs, buf.buf))
continue;
- if (existing_cruft_nr >= existing->cruft_packs.nr)
- BUG("too many cruft packs (found %"PRIuMAX", but knew "
- "of %"PRIuMAX")",
- (uintmax_t)existing_cruft_nr + 1,
- (uintmax_t)existing->cruft_packs.nr);
- existing_cruft[existing_cruft_nr++] = p;
- }
-
- QSORT(existing_cruft, existing_cruft_nr, existing_cruft_pack_cmp);
-
- for (i = 0; i < existing_cruft_nr; i++) {
- size_t proposed;
-
- p = existing_cruft[i];
- proposed = st_add(total_size, p->pack_size);
-
- if (proposed <= max_size) {
- total_size = proposed;
+ if (p->pack_size < max_size) {
fprintf(in, "-%s\n", pack_basename(p));
} else {
retain_cruft_pack(existing, p);
@@ -1086,7 +1053,6 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size,
existing->non_kept_packs.items[i].string);
strbuf_release(&buf);
- free(existing_cruft);
}
static int write_cruft_pack(const struct pack_objects_args *args,
diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh
index 959e6e26488..5a76b541ddd 100755
--- a/t/t7704-repack-cruft.sh
+++ b/t/t7704-repack-cruft.sh
@@ -194,48 +194,6 @@ test_expect_success '--max-cruft-size combines existing packs when below thresho
)
'
-test_expect_success '--max-cruft-size combines smaller packs first' '
- git init max-cruft-size-consume-small &&
- (
- cd max-cruft-size-consume-small &&
-
- test_commit base &&
- git repack -ad &&
-
- cruft_foo="$(generate_cruft_pack foo 524288)" && # 0.5 MiB
- cruft_bar="$(generate_cruft_pack bar 524288)" && # 0.5 MiB
- cruft_baz="$(generate_cruft_pack baz 1048576)" && # 1.0 MiB
- cruft_quux="$(generate_cruft_pack quux 1572864)" && # 1.5 MiB
-
- test-tool pack-mtimes "$(basename $cruft_foo)" >expect.raw &&
- test-tool pack-mtimes "$(basename $cruft_bar)" >>expect.raw &&
- sort expect.raw >expect.objects &&
-
- # repacking with `--max-cruft-size=2M` should combine
- # both 0.5 MiB packs together, instead of, say, one of
- # the 0.5 MiB packs with the 1.0 MiB pack
- ls $packdir/pack-*.mtimes | sort >cruft.before &&
- git repack -d --cruft --max-cruft-size=2M &&
- ls $packdir/pack-*.mtimes | sort >cruft.after &&
-
- comm -13 cruft.before cruft.after >cruft.new &&
- comm -23 cruft.before cruft.after >cruft.removed &&
-
- test_line_count = 1 cruft.new &&
- test_line_count = 2 cruft.removed &&
-
- # the two smaller packs should be rolled up first
- printf "%s\n" $cruft_foo $cruft_bar | sort >expect.removed &&
- test_cmp expect.removed cruft.removed &&
-
- # ...and contain the set of objects rolled up
- test-tool pack-mtimes "$(basename $(cat cruft.new))" >actual.raw &&
- sort actual.raw >actual.objects &&
-
- test_cmp expect.objects actual.objects
- )
-'
-
test_expect_success 'setup --max-cruft-size with freshened objects' '
git init max-cruft-size-freshen &&
(
--
2.49.0.rc0.2.gc0c926adde2
