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

[Jeff King <peff@peff.net>]

On Fri, Aug 14, 2026 at 09:49:26AM +0200, Patrick Steinhardt wrote:

> On Thu, Aug 13, 2026 at 12:15:25PM -0400, Jeff King wrote:
> > On Thu, Aug 13, 2026 at 09:35:49AM +0200, Patrick Steinhardt wrote:
> > 
> > > 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.
> > 
> > Don't we already use such a hashmap via packfile_store_add_pack() and
> > packfile_store_load_pack()? That comes from ec48540fe8 (packfile.c:
> > speed up loading lots of packfiles, 2019-11-27) and is how we know that
> > this "is_new" flag is true (otherwise we'd get duplicates during
> > "reprepare" operations).
> 
> That's a good point, we indeed do! Maybe it would make sense then to
> remove that map from the packfile store and instead move it into the
> packfile list to make it more generally useful.

The map protects more than just adding to the list; it avoids all of
add_packed_git(), which allocates and does a bunch of stat() calls.  So
it couldn't just be a check in packfile_list_append(), but would have to
be a separate existence check well before that.

That's not impossible, but it would be a lot easier to see what
generalized pattern would be most useful if there were more than one
caller of packfile_list_append(). ;)

-Peff