From: Jeff King <peff@peff.net>
To: Patrick Steinhardt <ps@pks.im>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] packfile: fix perf regression with many packs
Date: Mon, 17 Aug 2026 03:21:13 -0400 [thread overview]
Message-ID: <20260817072113.GA690018@coredump.intra.peff.net> (raw)
In-Reply-To: <aoKZvxE8oP5B6O_4@pks.im>
On Mon, Aug 17, 2026 at 07:18:55AM +0200, Patrick Steinhardt wrote:
> > 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.
Ah, indeed. I see prepend calls sprinkled in some rather hot code paths,
including the MRU adjustment from find_pack_entry(). That is a possible
candidate for Dscho's clone slowdown[1].
But I don't think would not want to pay the cost for a hash de-dup
there. We are not adding a new pack at all, but just adjusting the
placement, and that should be a quick O(1) if we are using a
doubly-linked list.
It's harder to construct a synthetic test for prepending because of pack
locality. If two subsequent requests both try to move pack A to the
front of the list, the second prepend()'s removal operation will find
the pack at the front in essentially constant time.
But we can spread the history across packs like this (I recommend
running on a ram disk, otherwise the checkpoint sync() makes it take
forever):
git init
for i in $(seq 10000); do
echo "commit refs/heads/foo"
echo "committer <none@example.com> $i +0000"
echo "data <<EOF"
echo "commit message $i"
echo "EOF"
echo
echo checkpoint
done |
git -c fastimport.unpackLimit=0 fast-import
And then timing "git rev-list --count foo" is interesting as the number
of packs grows:
- 500: 18ms
- 1000: 34ms
- 2000: 103ms
- 4000: 374ms
- 8000: 1648ms
- 16000: 6351ms
You can see the quadratic growth taking over around 2000 packs. But I'm
not sure that is proving anything about list management. Lookup across
packs is linear, so this situation is inherently quadratic. I think you
could probably make an argument that the list management follows exactly
the same quadratic patterns, and thus the MRU optimizes the removal from
the prepend, too.
Still, it seems prudent for these MRU updates to use a constant-time
movement within the list, rather than an explicit duplicate check and
removal.
-Peff
next prev parent reply other threads:[~2026-08-17 7:21 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-12 19:11 [PATCH] packfile: fix perf regression with many packs Johannes Schindelin via GitGitGadget
2026-08-12 19:51 ` Junio C Hamano
2026-08-12 21:29 ` Jeff King
2026-08-13 7:35 ` Patrick Steinhardt
2026-08-13 8:25 ` Johannes Schindelin
2026-08-13 16:10 ` Jeff King
2026-08-13 8:26 ` Johannes Schindelin
2026-08-12 22:29 ` Ben Knoble
2026-08-13 9:04 ` Johannes Schindelin
2026-08-13 11:18 ` Ben Knoble
2026-08-13 7:35 ` Patrick Steinhardt
2026-08-13 9:20 ` Johannes Schindelin
2026-08-13 10:01 ` Patrick Steinhardt
2026-08-13 10:42 ` [PATCH] packfile: fix perf regression with many packsy Johannes Schindelin
2026-08-13 11:12 ` Patrick Steinhardt
2026-08-13 13:52 ` [PATCH] packfile: fix perf regression with many packs Junio C Hamano
2026-08-13 16:15 ` Jeff King
2026-08-14 7:49 ` Patrick Steinhardt
2026-08-14 16:55 ` Jeff King
2026-08-17 5:18 ` Patrick Steinhardt
2026-08-17 7:21 ` Jeff King [this message]
2026-08-13 14:56 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2026-08-14 7:50 ` Patrick Steinhardt
2026-08-14 14:46 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260817072113.GA690018@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox