Re: [PATCH v2 29/33] refs: resolve symbolic refs first

On Thu, May 12, 2016 at 03:45:28AM -0400, Jeff King wrote:

So I'd expect us to hit that lock_ref_for_update() for each of the new
refs. But then we end up in verify_refname_available_dir(), which wants
to read all of the loose refs again. So we end up with a quadratic
number of calls to read_ref_full().

I haven't found the actual bug yet. It may be something as simple as not
clearing REF_INCOMPLETE from the loose-ref cache when we ought to. But
that's a wild (optimistic) guess.

Ah, nope, nothing so simple.

It looks like we get in a chain of:

  1. we want to update a ref, so we end up in
     verify_refname_available_dir to make sure we can do so.

  2. that has to load all of the loose refs in refs/tags, which is
     expensive.

  3. we actually update the ref, which calls clear_loose_ref_cache().

And repeat. Each ref update does the expensive step 2, and it gets
larger and larger each time.

I understand why we need to verify_refname_available() on the packed
refs. But traditionally we would rely on EISDIR or EEXIST to detect
conflicts with the loose refs, rather than loading using our own cache.
So I guess that's the new thing that is causing a problem.

-Peff