Thread (24 messages) flat view 24 messages, 6 authors, 7d ago

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

From: Jeff King <hidden>
Date: 2026-08-17 07:21:21

On Mon, Aug 17, 2026 at 07:18:55AM +0200, Patrick Steinhardt wrote:
quoted
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 [off-list ref] $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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help