Git development
 help / color / mirror / Atom feed
From: Taylor Blau <ttaylorr@openai.com>
To: Elijah Newren <newren@gmail.com>
Cc: Derrick Stolee <stolee@gmail.com>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Jeff King <peff@peff.net>, Ted Nyman <tnyman@openai.com>,
	Elijah Newren <newren@github.com>
Subject: Re: [PATCH 2/4] pack-objects: ensure tree/tag closure with '--stdin-packs=follow'
Date: Thu, 1 Oct 2026 19:51:48 -0500	[thread overview]
Message-ID: <ar8AJLYZVb6sCIO-@com-79390> (raw)
In-Reply-To: <CABPp-BHE662t9aaNcZ4DZ+2AU_C7jR7_VyHZwt2Tm8JSPE3JZw@mail.gmail.com>

On Thu, Oct 01, 2026 at 04:22:11PM -0700, Elijah Newren wrote:
> With the oid_array and parent-first pack order, the blobs are visited
> as sub/a and sub/b, so sub/* -delta applies. With the oidset, the
> subtree is visited first and the blobs are seen as a and b, so one is
> delta-compressed. When the root is processed later, the subtree is
> already marked SEEN and is not revisited with the sub/ prefix.
>
> The oid_array does not manufacture parent-before-child ordering if the
> input pack itself has the subtree first; this path information is
> explicitly best-effort. But it preserves a useful order when one
> exists, whereas an oidset discards it.

Sure, though as Peff and I discussed elsewhere in the thread, there are
also situations where you can produce a sub-optimal pack even with
oid_array. That's because the namehash you get for a given tree object
depends on the path you took to get there.

So you can certainly come up with examples where the ordering of tree
objects in an array of extra roots produces a lesser-quality delta
selection than the same objects permuted into some different order.

The other thing to keep in mind is that, while there are clearly
trade-offs as we have discussed here, the oidset ensures that we don't
allocate memory wastefully when the same object is listed multiple times
as an extra root.

The other other thing to keep in mind is that the size of this set is
almost always going to be puny compared to the size of the overall pack.
These objects are merely meant to pull in the (likely) few objects that
need refreshed out of the cruft pack in order to ensure reachability
closure.

So I think it's clear that this is a trade-off, and neither decision
(oidset vs oid_array) is absolutely perfect for all cases. But on
balance I think that the trade-offs push us towards oidset much more
than they do towards oid_array.

Thanks,
Taylor

  reply	other threads:[~2026-10-02  0:51 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  1:28 [PATCH 0/4] repack: various corner cases for cruft-less MIDXs Taylor Blau
2026-09-30  1:28 ` [PATCH 1/4] pack-objects: introduce `stdin_packs_context` struct Taylor Blau
2026-09-30 17:42   ` Junio C Hamano
2026-10-01  3:13     ` Taylor Blau
2026-09-30  1:28 ` [PATCH 2/4] pack-objects: ensure tree/tag closure with '--stdin-packs=follow' Taylor Blau
2026-09-30 17:51   ` Junio C Hamano
2026-09-30 18:16   ` Derrick Stolee
2026-10-01  3:14     ` Taylor Blau
2026-10-01 23:22     ` Elijah Newren
2026-10-02  0:51       ` Taylor Blau [this message]
2026-10-02 23:02         ` Jeff King
2026-09-30 20:31   ` Jeff King
2026-10-01  3:18     ` Taylor Blau
2026-09-30  1:28 ` [PATCH 3/4] repack: retain cruft packs in MIDXs after incremental repacks Taylor Blau
2026-09-30 20:45   ` Jeff King
2026-10-01  3:21     ` Taylor Blau
2026-09-30  1:28 ` [PATCH 4/4] repack: retain cruft packs in MIDXs containing kept packs Taylor Blau
2026-09-30 20:53   ` Jeff King
2026-10-01  3:35     ` Taylor Blau
2026-09-30 20:55 ` [PATCH 0/4] repack: various corner cases for cruft-less MIDXs Jeff King
2026-10-01  3:37   ` Taylor Blau
2026-10-01  4:11 ` [PATCH v2 0/8] " Taylor Blau
2026-10-01  4:11   ` [PATCH v2 1/8] pack-objects: introduce `stdin_packs_context` struct Taylor Blau
2026-10-01  4:11   ` [PATCH v2 2/8] pack-objects: ensure tree/tag closure with '--stdin-packs=follow' Taylor Blau
2026-10-02 23:13     ` Jeff King
2026-10-03  0:55       ` Taylor Blau
2026-10-03  1:06         ` Jeff King
2026-10-01  4:11   ` [PATCH v2 3/8] repack: retain cruft packs in MIDXs after incremental repacks Taylor Blau
2026-10-01  4:11   ` [PATCH v2 4/8] repack: use a sorted list for explicitly kept packs Taylor Blau
2026-10-02 23:16     ` Jeff King
2026-10-01  4:11   ` [PATCH v2 5/8] repack: follow kept packs when omitting cruft from the MIDX Taylor Blau
2026-10-02 23:25     ` Jeff King
2026-10-03  0:50       ` Taylor Blau
2026-10-01  4:11   ` [PATCH v2 6/8] repack: track the preferred pack explicitly in MIDX write steps Taylor Blau
2026-10-02 23:28     ` Jeff King
2026-10-03  1:00       ` Taylor Blau
2026-10-03  1:07         ` Jeff King
2026-10-01  4:12   ` [PATCH v2 7/8] repack: defer allocating the append plan's write step Taylor Blau
2026-10-01  4:12   ` [PATCH v2 8/8] repack: include required packs in incremental MIDX writes Taylor Blau
2026-10-02 23:41     ` Jeff King
2026-10-03  1:01       ` Taylor Blau

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=ar8AJLYZVb6sCIO-@com-79390 \
    --to=ttaylorr@openai.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@github.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=stolee@gmail.com \
    --cc=tnyman@openai.com \
    /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