Git development
 help / color / mirror / Atom feed
From: Taylor Blau <ttaylorr@openai.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Ted Nyman <tnyman@openai.com>, Elijah Newren <newren@github.com>
Subject: Re: [PATCH v2 6/8] repack: track the preferred pack explicitly in MIDX write steps
Date: Fri, 2 Oct 2026 20:00:52 -0500	[thread overview]
Message-ID: <asBTxLW9j2AIVlxZ@com-79390> (raw)
In-Reply-To: <20261002232834.GE834759@coredump.intra.peff.net>

On Fri, Oct 02, 2026 at 07:28:34PM -0400, Jeff King wrote:
> On Wed, Sep 30, 2026 at 11:11:58PM -0500, Taylor Blau wrote:
>
> > A MIDX write step marks preferred packs in its string-list entries and
> > chooses the last marked entry when executing the step. That makes the
> > choice depend on list order, preventing the list from being sorted for
> > membership checks.
> >
> > Record the last candidate directly in the step, borrowing its name from
> > the write list. This preserves preferred-pack selection while allowing
> > the list to be sorted without changing that choice.
>
> This is certainly cleaner, though it looks like the existing code works
> by marking item->util and then doing a linear search for it. So wouldn't
> that work even after sorting?

It would if only one entry were marked, but we can mark several.

For example, when `repack_make_midx_compaction_plan()` folds multiple
MIDX layers into one via a WRITE step, it marks each layer's preferred pack
without clearing the earlier marks. The scan doesn't stop at the first
such mark, and the last marked entry wins.

So sorting would of course preserve the marks, but may change which one
comes last.

> > @@ -719,7 +713,7 @@ static int repack_make_midx_compaction_plan(struct repack_write_midx_opts *opts,
> >
> >  		item = string_list_append(&step.u.write, buf.buf);
> >  		if (p->multi_pack_index || i == opts->geometry->pack_nr - 1)
> > -			item->util = (void *)1; /* mark as preferred */
> > +			step.preferred_pack = item->string;
>
> I am certainly happy to see these gross casts go away, though.

Me too ;-).

Thanks,
Taylor

  reply	other threads:[~2026-10-03  1:00 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
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 [this message]
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=asBTxLW9j2AIVlxZ@com-79390 \
    --to=ttaylorr@openai.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@github.com \
    --cc=peff@peff.net \
    --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