Subject: bug#74243: [PATCH] Speed up vc-hg-state by treating ignored files as unregistered

From: Spencer Baugh <[email-masked]>
Date: Tue, 26 Nov 2024 18:32:38 -0500

Dmitry Gutov <[email-masked]> writes:
> Hi, just to interject a little.
>
> On 26/11/2024 09:52, Sean Whitton wrote:
>> (I think there's a mistake here: an ignored file is not a file "under
>> version control", so `unregistered' should say "not under version
>> control and not ignored".  Would you agree?)
>> Thanks for pointing out the involvement of find-file-hook and
>> after-save-hook.  The problem you describe is not at all Hg-specific:
>> vc-state gets called in a context where speed matters, but it's also the
>> primary entry point for any code that wants to know the state of a file,
>> some of which might care more about accuracy than speed.
>> To put it another way, the code assumes throughout that finding out
>> the
>> file state will always be fast.  But it also assumes the information is
>> accurate if present.  This makes me queasy about your original patch.
>> It does not seem wise to return something we don't know to be true only
>> on the basis that it all works out fine for now.
>
> This FR reminds me of a similar change in vc-git-state that we ended
> up installing in bug#11757 (in 2012). Then stayed with it until 2017
> when bug#19343 was filed and fixed (provided a recent enough Git is
> used) - see also the problem scenario described there.
>
> So it seems both a reasonable change and ultimately not
> ideal. Depending on how many users we think might be affected by
> performance here.
>
>> The 'nil' return value might provide us with a way out, however.
>> Could we add an optional argument to vc-state that means "just return
>> nil if finding out the state properly might be slow"?
>> Could we make vc-after-save and the relevant find-file-hook entry pass
>> that option through, and do something sensible with a nil return value?
>> If they get nil, they would clear out the saved property, and
>> possibly
>> update the mode line display to "????" or something.  Maybe we'd want a
>> user option (that could go in your large repo's .dir-locals.el, so it's
>> set-and-forget) to opt-in to not knowing the file state as often.
>
> A user option seems like an easier choice.
>
> Solutions that clear cache under some conditions or other tend to be
> more complex, and slow down at least some combined scenarios (e.g. one
> of my use cases is saving the buffer and having diff-hl-mode use its
> vc state from after-save-hook).

These are all reasonable concerns.

Since posting my original patch, though, I've heard from Hg developers
that they plan to eventually implement per-directory ignore files, like
in Git.  That would remove the original performance problem, so maybe
this is not so important.

That being said, it's still sad that the vc hooks in find-file-hook and
after-save-hook can hurt performance so much.

It seems to me that the ideal outcome would be to support asynchronicity
in those hooks.  That would benefit all vc backends... though this is
perhaps quite difficult.

Maybe asynchronously populating the saved vc-state property would work?
With some clever usage of nil return values as Sean describes...