Re: [PATCH 1/2] repository: add core.indexLockTimeout

[Jun Wu <quark.zju@gmail.com>]

On Tue, 13 Oct 2026 17:40:01 -0400, D. Ben Knoble wrote:
> Hi Jun,

Hi Ben,

Thanks for taking a look. This area is a bit tricky, so let me try to
explain more clearly.

> Interesting. That must be the slow down you mentioned in your cover
> letter? I'm trying to make sense of this. I think you're saying that
> not having the index lock means status would "lose" refreshed info:
> we're not throwing away the *existing* stat info, just the ones we
> recomputed for this "git --no-optional-locks status" invocation,
> right?

Yes. A file's stat info might be updated without content change (e.g.
`touch`-ed) so the existing stat info can become outdated over time, and status
cannot use the stat fast path to confirm a file is not changed, so status falls
back to slow content check. Note: if the user runs a command like `add` that
writes the index, the performance can recover, but based on our experience,
users might not run such recovery commands for a long time, so they suffer from
slow status.

> I suppose I don't see how that causes things to slow down continually,
> but I bet I'm missing something :)

To give some (hopefully useful) examples,

1. https://github.com/facebook/sapling/issues/929#issuecomment-2628723586
   A user says "I use CLI exclusively so I would be really interested if
   --no-optional-locks was applied only to automation". The user was running
   Sapling in a `.git` repo. Sapling (back then) passed `--no-optional-locks`
   to every `git status` call. The user found `status` too slow to use, and
   hope that non-automation commands would not use `--no-optional-locks`.
   In other words, always using `--no-optional-locks` caused slowness.

2. https://github.com/facebook/sapling/commit/f11d555a3a237254e1788f9e9da40b37cd6e452f
   This is Sapling, but the concept (stat fast path) matches Git. Before this
   change, similar to Git, Sapling's status takes the wlock (akin to
   index.lock) in a best-effort way, and skip updating if it cannot get the
   lock.
   In production, we observed cases where `status` cannot get the lock for a
   long time, and the user's `status` remains slow (IIRC tens of seconds to
   minutes) without clear way to recover. This change forces a recovery path:
   if there are too many (>200) files with outdated `stat` info, then wait for
   the lock and update stats.

> It feels weird to me to fix mostly-invisible behavior from external
> tools with a config option, though. I think of config *mostly* as
> user-facing (whether that user is a client of the Git CLI or
> configuring a server repo), and I don't see how we could teach users
> to look for this or know what a reasonable setting should be for
> themselves :/ Further, we already have a lot of config options! (I'm
> guilty of adding one recently, heh.) It can definitely be overwhelming
> for users to think about all the options, and I suspect many just
> don't as a result.

I agree that users shouldn't need to know about extra options to get reasonable
behavior. In this case, I think the default (1s) is good, so users already
don't need to learn about it. The before/after matrix looks like:

    | Background status   |     Before         |      After      |
    |                     | slow   | cannot    | slow   | cannot |
    |                     | status | commit    | status | commit |
    --------------------------------------------------------------
    | --no-optional-locks | yes    | no        | yes    | no     |
    | (regular) status    | no     | sometimes | no     | rarely |

The bottom-right "no, rarely" + no need for `--no-optional-locks` seems to be
the ideal trade-off to me.

I think the same preference (good out-of-box) applies to `--no-optional-locks`,
too. I suspect that not a lot of tool authors know about `--no-optional-locks`.
https://github.com/facebook/sapling/commit/39380e1871d7c5f6f91ebb67c4efb7588eb9f65f
documented our version of "tool author doesn't know" - tool didn't set the
"is automation" env var, akin to forgetting `--no-optional-locks`. The repo in
question had a spiked ~40% `git checkout` failure rate.

> I applaud the effort for sensible defaults here, which I think will help.
>
> Anyway, maybe it's just me: if others are happy with this approach,
> great. I think I would prefer to see us try to address the underlying
> performance problems, though, if we can. (Maybe we can have some way
> for git-status to not throw away all its work, or some other good way
> to communicate to IDEs that want to poll?)

To recap, the user-facing problem is "commit/checkout" didn't work when
`status` was running, or `--no-optional-locks status` can be too slow.

By "address the underlying performance problems", do you mean:

1. Improve `status` performance so it takes index lock for a shorter duration.
   It helps, but as long as `status` takes the lock, there is a chance it
   conflicts with other commands. This series allows tools to drop
   `--no-optional-locks`, which indirectly helps `status` performance.
2. Improve `--no-optional-locks status` performance so it's fast.
   This sounds like "lock-free writes to the index", which might require
   a complex file format change.
3. Switch IDEs from polling `git status` to fsmonitor-style `status` notification.
   This helps. But similar to 1, reducing `status` frequency alone does not
   address the root issue.

For what it's worth, Mercurial and Sapling have waited for the wlock by
default for a long time, with default timeout set to 10 minutes. In our
experience that has worked well. The most common "waiting for lock" report we
see is an editor left open somewhere, which got rarer after we started printing
the process tree of the lock holder.

Thanks, Jun