From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
Elijah Newren <newren@gmail.com>,
Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v4 0/6] pack-objects: freshen objects with multi-cruft packs
Date: Tue, 11 Mar 2025 13:13:20 -0700 [thread overview]
Message-ID: <xmqqr0332un3.fsf@gitster.g> (raw)
In-Reply-To: <cover.1741648467.git.me@ttaylorr.com> (Taylor Blau's message of "Mon, 10 Mar 2025 20:21:35 -0400")
Taylor Blau <me@ttaylorr.com> writes:
> Here is a slightly larger reroll of my series to fix object freshening
> when using multi-cruft packs that I have been meaning to send for a
> couple of days.
>
> I realized after sending the last round that not only was the first
> commit from v1 flawed (for the reasons Patrick identified) but that
> there is currently no way to grow a new cruft pack past the configured
> limit.
>
> Independent of this series suppose for example that we have two 100 MiB
> packs, and the threshold is 200 MiB. We should able to in theory combine
> those packs together. But we can't! The largest pack we'll make is
> 199MiB (and change), since builtin/pack-objects.c::write_one() will
> refuse to write any object which would bust the limit given by
> --max-pack-size.
I am not sure why that is a problem. If we have many loose objects
and the threshold is set at 200, wouldn't we also give up at 199
plus a change when packing these loose objects into a pack? If the
last object that makes us bust the threshold is unusually large, say
50, we may give up at 150 plus a bit, unless we go back to the queue
and pick smaller objects among the remaining ones to fill the
remaining 50 minus a bit, and because we do not do that to enforce
max-pack-size, I am not sure how "give up just before the threshold"
is too bad and needs to be replaced with "give up just after".
Or is the problem that the threshold is applied differently based on
where the objects come from? E.g., packing many loose objects would
stop just after, but repacking from cruft would stop just before, or
something? If the problem is that we are inconsistent, then I would
understand that it may be good to make things consistent.
> This series resurrects the first patch from v1 after introducing a
> behavior change for 'git pack-objects --cruft --max-pack-size'. When
> given with '--cruft', '--max-pack-size' now allows pack-objects to grow
> a pack *just* past the given limit by at most one object.
And what happens when the last object appended is very large, like
70? Would we end up with 270 when the threshold says 200?
I still am not getting what you are trying to explain in the above
two paragraphs, but in general, "give up just before" would be a
better choice than "give up just after", exactly because the threshold
we are letting the user to give is the maximum.
next prev parent reply other threads:[~2025-03-11 20:13 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-27 18:29 [PATCH 0/2] pack-objects: freshen objects with multi-cruft packs Taylor Blau
2025-02-27 18:29 ` [PATCH 1/2] builtin/repack.c: simplify cruft pack aggregation Taylor Blau
2025-02-27 19:23 ` Elijah Newren
2025-02-27 22:53 ` Taylor Blau
2025-02-28 7:52 ` Patrick Steinhardt
2025-03-04 21:52 ` Elijah Newren
2025-03-05 2:04 ` Junio C Hamano
2025-03-05 0:09 ` Taylor Blau
2025-02-27 18:29 ` [PATCH 2/2] builtin/pack-objects.c: freshen objects from existing cruft packs Taylor Blau
2025-02-27 19:26 ` Elijah Newren
2025-02-27 23:03 ` Taylor Blau
2025-02-27 19:28 ` [PATCH 0/2] pack-objects: freshen objects with multi-cruft packs Elijah Newren
2025-02-27 23:05 ` Taylor Blau
2025-03-04 21:35 ` [PATCH v2 " Taylor Blau
2025-03-04 21:35 ` [PATCH v2 1/2] builtin/repack.c: simplify cruft pack aggregation Taylor Blau
2025-03-04 21:35 ` [PATCH v2 2/2] builtin/pack-objects.c: freshen objects from existing cruft packs Taylor Blau
2025-03-04 22:55 ` [PATCH v2 0/2] pack-objects: freshen objects with multi-cruft packs Elijah Newren
2025-03-05 0:06 ` Taylor Blau
2025-03-05 0:13 ` Taylor Blau
2025-03-05 0:15 ` [PATCH v3 0/1] " Taylor Blau
2025-03-05 0:15 ` [PATCH v3 1/1] builtin/pack-objects.c: freshen objects from existing cruft packs Taylor Blau
2025-03-06 10:31 ` Patrick Steinhardt
2025-03-13 17:32 ` Taylor Blau
2025-03-06 10:31 ` [PATCH v3 0/1] pack-objects: freshen objects with multi-cruft packs Patrick Steinhardt
2025-03-11 0:21 ` [PATCH v4 0/6] " Taylor Blau
2025-03-11 0:21 ` [PATCH v4 1/6] t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests Taylor Blau
2025-03-11 0:21 ` [PATCH v4 2/6] t7704-repack-cruft.sh: consolidate `write_blob()` Taylor Blau
2025-03-11 0:21 ` [PATCH v4 3/6] t/lib-cruft.sh: extract some cruft-related helpers Taylor Blau
2025-03-11 0:21 ` [PATCH v4 4/6] pack-objects: generate cruft packs at most one object over threshold Taylor Blau
2025-03-11 21:59 ` Junio C Hamano
2025-03-12 15:22 ` Taylor Blau
2025-03-12 18:26 ` Junio C Hamano
2025-03-12 19:02 ` Taylor Blau
2025-03-12 19:13 ` Elijah Newren
2025-03-12 19:33 ` Taylor Blau
2025-03-12 20:43 ` Junio C Hamano
2025-03-12 20:49 ` Elijah Newren
2025-03-13 12:16 ` Junio C Hamano
2025-03-13 16:23 ` Elijah Newren
2025-03-13 17:06 ` Junio C Hamano
2025-03-11 0:21 ` [PATCH v4 5/6] builtin/repack.c: simplify cruft pack aggregation Taylor Blau
2025-03-11 0:21 ` [PATCH v4 6/6] builtin/pack-objects.c: freshen objects from existing cruft packs Taylor Blau
2025-03-11 20:13 ` Junio C Hamano [this message]
2025-03-12 15:33 ` [PATCH v4 0/6] pack-objects: freshen objects with multi-cruft packs Taylor Blau
2025-03-12 18:28 ` Junio C Hamano
2025-03-12 19:04 ` Taylor Blau
2025-03-12 19:46 ` Junio C Hamano
2025-03-12 19:52 ` Taylor Blau
2025-03-13 17:17 ` Junio C Hamano
2025-03-13 17:35 ` Taylor Blau
2025-03-13 6:29 ` Jeff King
2025-03-13 15:12 ` Junio C Hamano
2025-03-13 18:09 ` [PATCH v5] builtin/pack-objects.c: freshen objects from existing cruft packs Taylor Blau
2025-03-13 18:41 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqr0332un3.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).