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