Re: [PATCH] packfile: fix perf regression with many packsy

Hi Patrick,

On Thu, 13 Aug 2026, Patrick Steinhardt wrote:

> On Thu, Aug 13, 2026 at 11:20:11AM +0200, Johannes Schindelin wrote:
> > On Thu, 13 Aug 2026, Patrick Steinhardt wrote:
> > > I wonder whether we should slightly reformulate this and rename `is_new`
> > > to `accept_duplicates`. Because ultimately, that is what we're doing
> > > now: instead of ensuring that the packfile is unique in the list, we
> > > just don't care and just append the entry to the list.
> > 
> > Hmm. I don't quite agree, we're _not_ accepting duplicates. We know that
> > those packfiles _cannot_ be duplicates.
> 
> I know that we're not, but this is only because the caller knows that
> the packs are new. Seen outside that context though the new parameter
> really just tells us whether or not we want to deduplicate packs or not.
> 
> Anyway, I'm splitting hairs and I won't insist on a change here.

You do have a point, though, `is_new` is too narrow. How about
`skip_dup_check`?

> > > An alternative would be to use a hashmap here that tracks the packs that
> > > have already been added. It has the advantage that it also covers the
> > > `prepend()` operation and that callers don't have to be aware of this
> > > mechanism at all. Furthermore, moving preexisting entries to the back or
> > > front could become O(logn) if the list was doubly-linked. We do this
> > > operation quite often to re-sort entries in the list when looking up
> > > objects.
> > 
> > Indeed, that was my initial reaction, too. I was well on my way to start
> > writing a hashmap-based fix when the AI assistant pointed out that no
> > duplicates could possibly exist yet.
> > 
> > > Overall though I'm not quite sure whether the added complexity would be
> > > worth it, see below patch.
> > 
> > Wow, you got a lot further than I did! And yes, I agree that we do not
> > (yet?) need to deal with the added complexity.
> 
> I may want to pursue this patch anyway, as I think that the reordering
> would be sped up by that change quite signifcantly. And that would make
> a difference indeed when you have 38k packfiles, at least when you
> assume that objects are evenly distributed across all of those and that
> we perform reads of random objects.
> 
> I could do that tomorrow, and in that case it'd supersede your patch.

I don't think that it would _quite_ supersede this patch. Sure, while
searching through a hashset instead of a single-linked list is faster, it
is not as fast as skipping the search altogether.

Ciao,
Johannes

> But I'm also happy to have this improvement here land first and then
> I'll pursue this change eventually.
> 
> Thanks!
> 
> Patrick
>