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

Jeff King <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.