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

On Fri, Aug 14, 2026 at 12:55:46PM -0400, Jeff King wrote:
> 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(). ;)

We only have a single caller that appends, but we have some more that
use `packfile_list_prepend()`. And there we basically have the same
problem.

Patrick