Package: emacs;
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.
View this message in rfc822 format
From: Michael Albinus <michael.albinus <at> gmx.de> To: Dmitry Gutov <dgutov <at> yandex.ru> Cc: 11757 <at> debbugs.gnu.org Subject: bug#11757: Acknowledgement (24.1.50; vc-git calls `process-file' too many times) Date: Fri, 29 Jun 2012 15:46:33 +0200
Dmitry Gutov <dgutov <at> 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)
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.