GNU bug report logs -
#11757
24.1.50; vc-git calls `process-file' too many times
Previous Next
Reported by: Dmitry Gutov <dgutov <at> yandex.ru>
Date: Thu, 21 Jun 2012 02:17:02 UTC
Severity: normal
Found in version 24.1.50
Done: Dmitry Gutov <dgutov <at> yandex.ru>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
Sorry, forgot to cc.
On 01.07.2012 13:58, Michael Albinus wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>>>> I'm not sure what we should do. Call (vc-mode-line) anyway? That would work.
>>>
>>> Nope. This is expansive, because it recomputes `vc-working-revision'. We
>>> would loose all improvements from using the cache.
>>
>> I don't think so.
>>
>> If we hadn't reset all properties in vc-before-save (file existed),
>> nothing changes.
>> If we did reset them, then yes, vc-working-revision will recompute
>> vc-working-revision property, but only once after the reset.
>> Which is what we want to do anyway, since the file's state has
>> changed, and the working revision could have changed as well, so we
>> need to know them to update mode-line.
>
> Likely, the best option is to call (vc-registered file) after clearing
> the file cache. This recomputes the 'vc-backend property as well, what
> we want.
Have you tried this? (vc-backend file) gets called just a bit later, in
`vc-after-save`, and it calls `vc-registered' in turn, but since the
file is no longer registered with Git, it returns nil, and the mode-line
is not updated.
Also, I don't think we should call (vc-backend file) before the file is
written to disk.
> Calling (vc-mode-line file) at this point would be for the side-effect
> of that function, which is bad in my experience. It would harden
> maintenance, 'cause nobody will know why we want to refresh the modeline
> at this point.
I meant, always calling (vc-mode-line file) in `vc-after-save', moving
it outside of the (and backend ...) check.
This does have a performance downside with non-VC file buffers, of a
couple additional function calls, 'vc-backend cache lookup, and
unconditional mode-line update.
So from this standpoint, saving and restoring 'vc-backend value around
clearing props in `vc-before-save' might indeed be the best solution.
>> By the way, this last patch I sent doesn't help if the user just
>> removed the file from repository while leaving it on disk (git rm
>> --cached ... && git commit ..., for example), but whatever.
>
> If we use the cache, there will always be a constellation that the cache
> is stale due to external operations. As Stefan said, this is mostly
> uncritical.
I'm bringing attention to these cases because they could be the actual
benefit of your patch (caching 'git-registered) over my initial patch
(not calling `vc-git-registered' from `vc-git-state' at all).
The original code supported these edge cases, at the cost of additional
process call.
-- Dmitry
This bug report was last modified 12 years and 362 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.