Subject: Re: bug#11757: Acknowledgement (24.1.50; vc-git calls `process-file' too many times)

From: Dmitry Gutov <[email-masked]>
Date: Thu, 28 Jun 2012 04:51:54 +0400

Hi Michael,

This little patch shaves 2 `process-file' invocations from both 
`vc-find-file-hook' and `vc-after-save'.

It's not fully backward-compatible (it breaks when a previously 
registered file became unregistered), but I think it's a good tradeoff.

VC doesn't handle all cases of "outside interference" anyway: for 
example, the cached return value of `vc-working-revision' is invalidated 
only after the file is checked in, moved, or deleted, not after each 
save, and switching to another branch in Git is a much more common 
occurrence.

A more complex solution, though also applicable to vc-hg, would be to 
move most of the code from `vc-git-registered' to `vc-git-state', then 
modify `vc-git-registered' and `vc-hg-registered' to call `vc-state' 
(for caching) and then interpret its return value (what 
`vc-hg-registered' already does). Since `vc-registered' calls 
`vc-xx-registered' functions when selecting the backend, we'd also need 
to clear the cached 'vc-state value after each call with negative result.

This would mean +1 `process-file' invocation with Git in 
`vc-after-save', compared to the attached patch, but -1 with Hg in 
`vc-find-file-hook'.

On 26.06.2012 16:14, Michael Albinus wrote:
> Dmitry Gutov <[email-masked]> writes:
>
>> After setting vc-git-program explicitly to "C:/Program Files
>> (x86)/Git/bin/git.exe" (by default, it was calling "C:/Program Files
>> (x86)/Git/cmd/git.CMD"), the average time of git command calls went
>> down from 0.2s to 0.03s, so the problem with Git is about 7 times less
>> important now.
>
> If the git repository is remote (accessed via Tramp), this is still a
> serious issue. I'm plagued by this every single day. Less `process-file'
> invocations would be more than welcome. Maybe vc-git can cache some results?
>
>> -- Dmitry
>
> Best regards, Michael.
>
>

>From 8035692de90e463aa83576d68c6d008f77362fdc Mon Sep 17 00:00:00 2001
From: Dmitry Gutov <[email-masked]>
Date: Thu, 28 Jun 2012 04:01:06 +0400
Subject: [PATCH 4/4] * vc-git.el (vc-git-state): Don't call
 `vc-git-registered', it's expensive.

It's possible that file's status had changed to 'unregistered' since it was
opened, but is checking this worth another process call?
Note that if this is the case, `vc-next-action' won't add the file now.

(vc-git-mode-line-string): Call `vc-working-revision' instead of
`vc-git-working-revision' because it caches the result.
---
 lisp/vc/vc-git.el |   38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 63243cd..329a950 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -215,23 +215,25 @@ matching the resulting Git log output, and KEYWORDS is a list of
   ;; is direct ancestor of corresponding upstream branch, and the file
   ;; was modified upstream.  But we can't check that without a network
   ;; operation.
-  (if (not (vc-git-registered file))
-      'unregistered
-    (let ((diff (vc-git--run-command-string
-                 file "diff-index" "-p" "--raw" "-z" "HEAD" "--")))
-      (if (and diff
-	       (string-match ":[0-7]\\{6\\} [0-7]\\{6\\} [0-9a-f]\\{40\\} [0-9a-f]\\{40\\} \\([ADMUT]\\)\0[^\0]+\0\\(.*\n.\\)?"
-			     diff))
-          (let ((diff-letter (match-string 1 diff)))
-            (if (not (match-beginning 2))
-                ;; Empty diff: file contents is the same as the HEAD
-                ;; revision, but timestamps are different (eg, file
-                ;; was "touch"ed).  Update timestamp in index:
-                (prog1 'up-to-date
-                  (vc-git--call nil "add" "--refresh" "--"
-                                (file-relative-name file)))
-              (vc-git--state-code diff-letter)))
-	(if (vc-git--empty-db-p) 'added 'up-to-date)))))
+  ;; This assumes that status is known to be not `unregistered' because
+  ;; we've been successfully dispatched here from `vc-state', that
+  ;; means `vc-git-registered' returned t earlier once.  Not 100% accurate,
+  ;; but close enough.
+  (let ((diff (vc-git--run-command-string
+               file "diff-index" "-p" "--raw" "-z" "HEAD" "--")))
+    (if (and diff
+             (string-match ":[0-7]\\{6\\} [0-7]\\{6\\} [0-9a-f]\\{40\\} [0-9a-f]\\{40\\} \\([ADMUT]\\)\0[^\0]+\0\\(.*\n.\\)?"
+                           diff))
+        (let ((diff-letter (match-string 1 diff)))
+          (if (not (match-beginning 2))
+              ;; Empty diff: file contents is the same as the HEAD
+              ;; revision, but timestamps are different (eg, file
+              ;; was "touch"ed).  Update timestamp in index:
+              (prog1 'up-to-date
+                (vc-git--call nil "add" "--refresh" "--"
+                              (file-relative-name file)))
+            (vc-git--state-code diff-letter)))
+      (if (vc-git--empty-db-p) 'added 'up-to-date))))
 
 (defun vc-git-working-revision (_file)
   "Git-specific version of `vc-working-revision'."
@@ -248,7 +250,7 @@ matching the resulting Git log output, and KEYWORDS is a list of
 
 (defun vc-git-mode-line-string (file)
   "Return a string for `vc-mode-line' to put in the mode line for FILE."
-  (let* ((branch (vc-git-working-revision file))
+  (let* ((branch (vc-working-revision file 'Git))
          (def-ml (vc-default-mode-line-string 'Git file))
          (help-echo (get-text-property 0 'help-echo def-ml)))
     (if (zerop (length branch))
-- 
1.7.10.msysgit.1