Thread (31 messages) 31 messages, 3 authors, 2025-03-13

Re: [PATCH 1/2] builtin/repack.c: simplify cruft pack aggregation

From: Elijah Newren <hidden>
Date: 2025-03-04 21:53:10

On Thu, Feb 27, 2025 at 11:52 PM Patrick Steinhardt [off-list ref] wrote:
On Thu, Feb 27, 2025 at 01:29:28PM -0500, Taylor Blau wrote:
quoted
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.
Hm. So the result would be a new set of packfiles where each of them is
smaller than the threshold, right?
Are you assuming there's only one threshold, or that --max-pack-size
== --max-cruft-size?

I read this assuming --max-pack-size >> --max-cruft-size, so the odds
that the N packs smaller than --max-cruft-size add up to more than
--max-pack-size is small -- but even if it does happen, it just
results in the cruft packs being split out into a couple packs.
Wouldn't that mean that the next time
we'll again do the same thing and try to combine the new set of cruft
packs into one, and basically never arrive at a state where we don't
touch the cruft packs anymore?
This would be a risk if we allow --max-cruft-size to approach or be
equal to --max-pack-size.  (And if --max-pack-size is less than
--max-cruft-size, then we'll perversely split into even more cruft
packs rather than combining as intended.)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help