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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66364 in the body.
You can then email your comments to 66364 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.1.50; vc-hg-registered/state are slow on directories of large repos
Date: Thu, 05 Oct 2023 11:33:32 -0400

vc-hg-state (and vc-hg-registered, which calls vc-hg-state) are slow
when run on directories of large repos.  In fact they're O(N) in the
number of files in the repo.

This is because vc-hg-state-slow runs "hg status" on directories in a
mode which lists all the files in the directory, and then it parses that
list.  Which is pointlessly slow.

However, Hg (like git) does not actually track directories.  So in the
end vc-hg-state on a directory should always be returning 'unregistered,
which matches the existing behavior of vc-hg (vc-hg-registered always
returned nil for directories) and is much faster.  (It also matches what
vc-git does.)

A patch to do this will follow

Message #8 received at 66364 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: 66364 <at> debbugs.gnu.org
Subject: Re: bug#66364: 29.1.50; vc-hg-registered/state are slow on
 directories of large repos
Date: Thu, 05 Oct 2023 11:36:08 -0400

[0001-Optimize-vc-hg-state-for-directories.patch (text/x-patch, inline)]

From 5573b678f816f81623deb7ebde66dfd3ebe92355 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 5 Oct 2023 11:35:25 -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 unregistered for directories.
---
 lisp/vc/vc-hg.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index c3e563a1f10..9a30706f519 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -216,8 +216,10 @@ 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)))
+  (if (file-directory-p file)
+      'unregistered
+    (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.39.3

Message #11 received at 66364 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, 66364 <at> debbugs.gnu.org
Subject: Re: bug#66364: 29.1.50; vc-hg-registered/state are slow on
 directories of large repos
Date: Thu, 5 Oct 2023 19:09:52 +0300

On 05/10/2023 18:36, Spencer Baugh wrote:
>  From 5573b678f816f81623deb7ebde66dfd3ebe92355 Mon Sep 17 00:00:00 2001
> From: Spencer Baugh<sbaugh <at> janestreet.com>
> Date: Thu, 5 Oct 2023 11:35:25 -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 unregistered for directories.
> ---
>   lisp/vc/vc-hg.el | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
> index c3e563a1f10..9a30706f519 100644
> --- a/lisp/vc/vc-hg.el
> +++ b/lisp/vc/vc-hg.el
> @@ -216,8 +216,10 @@ 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)))
> +  (if (file-directory-p file)
> +      'unregistered
> +    (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."

Perhaps we should just follow the example of vc-git-registered and 
return nil.

Could you mention which code calls 'registered' on a directory, though? 
If it's in-tree, that's probably a bug too.

Message #14 received at 66364 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66364 <at> debbugs.gnu.org
Subject: Re: bug#66364: 29.1.50; vc-hg-registered/state are slow on
 directories of large repos
Date: Thu, 05 Oct 2023 12:19:37 -0400

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 05/10/2023 18:36, Spencer Baugh wrote:
>>  From 5573b678f816f81623deb7ebde66dfd3ebe92355 Mon Sep 17 00:00:00 2001
>> From: Spencer Baugh<sbaugh <at> janestreet.com>
>> Date: Thu, 5 Oct 2023 11:35:25 -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 unregistered for
>> directories.
>> ---
>>   lisp/vc/vc-hg.el | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
>> index c3e563a1f10..9a30706f519 100644
>> --- a/lisp/vc/vc-hg.el
>> +++ b/lisp/vc/vc-hg.el
>> @@ -216,8 +216,10 @@ 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)))
>> +  (if (file-directory-p file)
>> +      'unregistered
>> +    (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."
>
> Perhaps we should just follow the example of vc-git-registered and
> return nil.

Also fine by me.

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

Message #17 received at 66364 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 66364 <at> debbugs.gnu.org
Subject: Re: bug#66364: 29.1.50; vc-hg-registered/state are slow on
 directories of large repos
Date: Thu, 5 Oct 2023 20:06:28 +0300

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

Message #24 received at 66364 <at> debbugs.gnu.org (full text, mbox):

From: sbaugh <at> catern.com
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 66364 <at> debbugs.gnu.org
Subject: Re: bug#66364: 29.1.50; vc-hg-registered/state are slow on
 directories of large repos
Date: Fri, 13 Oct 2023 01:09:26 +0000 (UTC)

[Message part 1 (text/plain, inline)]

Dmitry Gutov <dmitry <at> 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.

[0001-Optimize-vc-hg-state-for-directories.patch (text/x-patch, inline)]

From 39a453555811aad18add3272d2325c69785f89c0 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> 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

Message #29 received at 66364-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: sbaugh <at> catern.com
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 66364-done <at> debbugs.gnu.org
Subject: Re: bug#66364: 29.1.50; vc-hg-registered/state are slow on
 directories of large repos
Date: Sat, 14 Oct 2023 20:02:16 +0300

On 13/10/2023 04:09, sbaugh <at> catern.com wrote:
> 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.

Certainly.

Installed both patches, thank you, and closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.