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

[Michael Albinus]

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)