Re: [PATCH 1/2] builtin/repack.c: simplify cruft pack aggregation
- From
Elijah Newren <newren@gmail.com>
- Date
- Mar 4, 2025, 21:52 UTC
- Message-ID
- <CABPp-BGnbQdeSC=XUwTWOnr8=PiGMnt1Q0OXkUAzD=ZiaZoUhw@mail.gmail.com>
- In-Reply-To
- <Z8FrIfAj_oVwxz3X@pks.im>
On Thu, Feb 27, 2025 at 11:52 PM Patrick Steinhardt <ps@pks.im> wrote:
Show 34 quoted lines
>
> On Thu, Feb 27, 2025 at 01:29:28PM -0500, Taylor Blau wrote:
> > 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.)