* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos @ 2023-10-05 15:33 Spencer Baugh 2023-10-05 15:36 ` Spencer Baugh 0 siblings, 1 reply; 7+ messages in thread From: Spencer Baugh @ 2023-10-05 15:33 UTC (permalink / raw) To: 66364 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos 2023-10-05 15:33 bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos Spencer Baugh @ 2023-10-05 15:36 ` Spencer Baugh 2023-10-05 16:09 ` Dmitry Gutov 0 siblings, 1 reply; 7+ messages in thread From: Spencer Baugh @ 2023-10-05 15:36 UTC (permalink / raw) To: 66364 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: 0001-Optimize-vc-hg-state-for-directories.patch --] [-- Type: text/x-patch, Size: 1314 bytes --] From 5573b678f816f81623deb7ebde66dfd3ebe92355 Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos 2023-10-05 15:36 ` Spencer Baugh @ 2023-10-05 16:09 ` Dmitry Gutov 2023-10-05 16:19 ` Spencer Baugh 0 siblings, 1 reply; 7+ messages in thread From: Dmitry Gutov @ 2023-10-05 16:09 UTC (permalink / raw) To: Spencer Baugh, 66364 On 05/10/2023 18:36, Spencer Baugh wrote: > From 5573b678f816f81623deb7ebde66dfd3ebe92355 Mon Sep 17 00:00:00 2001 > From: Spencer Baugh<sbaugh@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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos 2023-10-05 16:09 ` Dmitry Gutov @ 2023-10-05 16:19 ` Spencer Baugh 2023-10-05 17:06 ` Dmitry Gutov 0 siblings, 1 reply; 7+ messages in thread From: Spencer Baugh @ 2023-10-05 16:19 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 66364 Dmitry Gutov <dmitry@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@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) ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos 2023-10-05 16:19 ` Spencer Baugh @ 2023-10-05 17:06 ` Dmitry Gutov 2023-10-13 1:09 ` sbaugh 0 siblings, 1 reply; 7+ messages in thread From: Dmitry Gutov @ 2023-10-05 17:06 UTC (permalink / raw) To: Spencer Baugh; +Cc: 66364 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos 2023-10-05 17:06 ` Dmitry Gutov @ 2023-10-13 1:09 ` sbaugh 2023-10-14 17:02 ` Dmitry Gutov 0 siblings, 1 reply; 7+ messages in thread From: sbaugh @ 2023-10-13 1:09 UTC (permalink / raw) To: Dmitry Gutov; +Cc: Spencer Baugh, 66364 [-- Attachment #1: Type: text/plain, Size: 2429 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Optimize-vc-hg-state-for-directories.patch --] [-- Type: text/x-patch, Size: 1292 bytes --] 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos 2023-10-13 1:09 ` sbaugh @ 2023-10-14 17:02 ` Dmitry Gutov 0 siblings, 0 replies; 7+ messages in thread From: Dmitry Gutov @ 2023-10-14 17:02 UTC (permalink / raw) To: sbaugh; +Cc: Spencer Baugh, 66364-done On 13/10/2023 04:09, sbaugh@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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-14 17:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-05 15:33 bug#66364: 29.1.50; vc-hg-registered/state are slow on directories of large repos Spencer Baugh 2023-10-05 15:36 ` Spencer Baugh 2023-10-05 16:09 ` Dmitry Gutov 2023-10-05 16:19 ` Spencer Baugh 2023-10-05 17:06 ` Dmitry Gutov 2023-10-13 1:09 ` sbaugh 2023-10-14 17:02 ` Dmitry Gutov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.