bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos

[sbaugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 05/10/2023 19:19, Spencer Baugh wrote:
>>> Could you mention which code calls 'registered' on a directory,
>>> though? If it's in-tree, that's probably a bug too.
>> vc-root-diff.  Here's the trace:
>> * vc-hg-registered("~/test-hg-repos/empty/")
>>    apply(vc-hg-registered "~/test-hg-repos/empty/")
>>    vc-call-backend(Hg registered "~/test-hg-repos/empty/")
>>    #f(compiled-function (b) #<bytecode -0x10c889a4506dd5b1>)(Hg)
>>    mapc(#f(compiled-function (b) #<bytecode -0x10c889a4506dd5b1>) (FE RCS 
>> CVS SVN SCCS SRC Bzr Git Hg))
>>    vc-registered("~/test-hg-repos/empty/")
>>    vc-backend("~/test-hg-repos/empty/")
>>    vc-working-revision("~/test-hg-repos/empty/")
>>    vc-root-diff(nil t)
>>    funcall-interactively(vc-root-diff nil t)
>>    call-interactively(vc-root-diff nil nil)
>>    command-execute(vc-root-diff)
>
> Huh, it actually looks like that call is unnecessary (the result is
> unused). These lines just come from 2009, mostly unchanged:
>
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index d3e53858c16..e3b1f7fafda 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -2086,7 +2086,7 @@ vc-root-diff
>      (vc-maybe-buffer-sync not-urgent)
>      (let ((backend (vc-deduce-backend))
>         (default-directory default-directory)
> -       rootdir working-revision)
> +       rootdir)
>        (if backend
>         (setq rootdir (vc-call-backend backend 'root default-directory))
>       (setq rootdir (read-directory-name "Directory for VC root-diff: "))
> @@ -2094,14 +2094,13 @@ vc-root-diff
>       (if backend
>           (setq default-directory rootdir)
>         (error "Directory is not version controlled")))
> -      (setq working-revision (vc-working-revision rootdir))
>        ;; relative to it.  Bind default-directory to the root directory
>        ;; here, this way the *vc-diff* buffer is setup correctly, so
>        ;; relative file names work.
>        (let ((default-directory rootdir))
>          (vc-diff-internal
> -         t (list backend (list rootdir) working-revision) nil nil
> +         t (list backend (list rootdir)) nil nil
>           (called-interactively-p 'interactive))))))
>
>  ;;;###autoload

Yes, I would be quite happy with deleting these unused lines.  Although
I expect it's still nice to have the same optimization that vc-git does.

BTW, here's a version of my patch which follows vc-git and just returns
nil for directories.

>From 39a453555811aad18add3272d2325c69785f89c0 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Thu, 12 Oct 2023 21:06:53 -0400
Subject: [PATCH] Optimize vc-hg-state for directories

Directories are never tracked in hg, so it's pointless to run
vc-hg-state on them.  And, in fact, our implementation previously
would list all the files contained in the directory and then parse
that in Emacs, which is very slow in large repos.

Let's just use the knowledge that directories aren't tracked in hg,
and skip running hg entirely.

* lisp/vc/vc-hg.el (vc-hg-state): Return nil for
directories.  (Bug#66364)
---
 lisp/vc/vc-hg.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index c3e563a1f10..f2ee9ef35e4 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -216,8 +216,9 @@ vc-hg-registered
 
 (defun vc-hg-state (file)
   "Hg-specific version of `vc-state'."
-  (let ((state (vc-hg-state-fast file)))
-    (if (eq state 'unsupported) (vc-hg-state-slow file) state)))
+  (unless (file-directory-p file)
+    (let ((state (vc-hg-state-fast file)))
+      (if (eq state 'unsupported) (vc-hg-state-slow file) state))))
 
 (defun vc-hg-state-slow (file)
   "Determine status of FILE by running hg."
-- 
2.41.0