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

[Michael Albinus <michael.albinus@gmx.de>]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi Michael,

Hi Dmitry,

> 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.

I don't know whether we shall break the functionality. Instead of, I've
appended a small patch, which uses the cache for vc-git-registered and
vc-git-root (additionally to your patch, which uses the cache of
vc-working-revision). This reduces already the number of process-file
invocations from 6 to 4, when openening a new file. And there's room for
other caches.

> 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 stale cache is bad, of course. We must carefully check, where a cached
value has to be invalidated. But why should vc-working-revision being
invalidated after saving? It is still the same, I believe. Switching to
another branch shall be observed by Emacs, 'cause there is another
version of the file on the disk, and Emacs warns you before editing.

> 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 I cannot answer (yet). We could apply this, when the other changes
work robustly.

> -- Dmitry

Best regards, Michael.

*** /usr/local/src/emacs/lisp/vc/vc-git.el.~108799~  2012-06-29 15:39:15.897374680 +0200
--- /usr/local/src/emacs/lisp/vc/vc-git.el	2012-06-29 15:24:04.389401221 +0200
***************
*** 173,200 ****

  (defun vc-git-registered (file)
    "Check whether FILE is registered with git."
!   (let ((dir (vc-git-root file)))
!     (when dir
!       (with-temp-buffer
! 	(let* (process-file-side-effects
! 	       ;; Do not use the `file-name-directory' here: git-ls-files
! 	       ;; sometimes fails to return the correct status for relative
! 	       ;; path specs.
! 	       ;; See also: http://marc.info/?l=git&m=125787684318129&w=2
! 	       (name (file-relative-name file dir))
! 	       (str (ignore-errors
! 		     (cd dir)
! 		     (vc-git--out-ok "ls-files" "-c" "-z" "--" name)
! 		     ;; If result is empty, use ls-tree to check for deleted
!                      ;; file.
! 		     (when (eq (point-min) (point-max))
! 		       (vc-git--out-ok "ls-tree" "--name-only" "-z" "HEAD"
!                                        "--" name))
! 		     (buffer-string))))
! 	  (and str
! 	       (> (length str) (length name))
! 	       (string= (substring str 0 (1+ (length name)))
! 			(concat name "\0"))))))))

  (defun vc-git--state-code (code)
    "Convert from a string to a added/deleted/modified state."
--- 173,203 ----

  (defun vc-git-registered (file)
    "Check whether FILE is registered with git."
!   (or (vc-file-getprop file 'git-registered)
!       (vc-file-setprop
!        file 'git-registered
!        (let ((dir (vc-git-root file)))
! 	 (when dir
! 	   (with-temp-buffer
! 	     (let* (process-file-side-effects
! 		    ;; Do not use the `file-name-directory' here: git-ls-files
! 		    ;; sometimes fails to return the correct status for relative
! 		    ;; path specs.
! 		    ;; See also: http://marc.info/?l=git&m=125787684318129&w=2
! 		    (name (file-relative-name file dir))
! 		    (str (ignore-errors
! 			   (cd dir)
! 			   (vc-git--out-ok "ls-files" "-c" "-z" "--" name)
! 			   ;; If result is empty, use ls-tree to check for deleted
! 			   ;; file.
! 			   (when (eq (point-min) (point-max))
! 			     (vc-git--out-ok "ls-tree" "--name-only" "-z" "HEAD"
! 					     "--" name))
! 			   (buffer-string))))
! 	       (and str
! 		    (> (length str) (length name))
! 		    (string= (substring str 0 (1+ (length name)))
! 			     (concat name "\0"))))))))))

  (defun vc-git--state-code (code)
    "Convert from a string to a added/deleted/modified state."
***************
*** 248,254 ****

  (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))
           (def-ml (vc-default-mode-line-string 'Git file))
           (help-echo (get-text-property 0 'help-echo def-ml)))
      (if (zerop (length branch))
--- 251,257 ----

  (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-working-revision file))
           (def-ml (vc-default-mode-line-string 'Git file))
           (help-echo (get-text-property 0 'help-echo def-ml)))
      (if (zerop (length branch))
***************
*** 959,965 ****
  (defun vc-git-extra-status-menu () vc-git-extra-menu-map)

  (defun vc-git-root (file)
!   (vc-find-root file ".git"))

  ;; Derived from `lgrep'.
  (defun vc-git-grep (regexp &optional files dir)
--- 962,969 ----
  (defun vc-git-extra-status-menu () vc-git-extra-menu-map)

  (defun vc-git-root (file)
!   (or (vc-file-getprop file 'git-root)
!       (vc-file-setprop file 'git-root (vc-find-root file ".git"))))

  ;; Derived from `lgrep'.
  (defun vc-git-grep (regexp &optional files dir)