* [PATCH] Speed up project-kill-buffers @ 2021-05-03 9:43 Philip Kaludercic 2021-05-03 12:46 ` Stefan Monnier 2021-05-03 21:43 ` Dmitry Gutov 0 siblings, 2 replies; 26+ messages in thread From: Philip Kaludercic @ 2021-05-03 9:43 UTC (permalink / raw To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 505 bytes --] Hi, I've noticed that sometimes project-kill-buffers is noticeably slow, and it seems like it's has to do with project--buffer-list working on remote files. The function goes through every buffer and calls (project-current), even if the buffer is related to a remote file that cannot be part of the current project. The patch I attach below is a simple fix to avoid checking files that cannot be part of the current project. Or are there any edge-cases that this code approach breaks? -- Philip K. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Avoid-Tramp-buffers-when-possible.patch --] [-- Type: text/x-diff, Size: 1246 bytes --] From 8b45502da8281826fa2da02a317546bc99f51069 Mon Sep 17 00:00:00 2001 From: Philip K <philipk@posteo.net> Date: Mon, 3 May 2021 11:35:41 +0200 Subject: [PATCH] Avoid Tramp buffers when possible * project.el (project--buffer-list): Add file-remote-p check --- lisp/progmodes/project.el | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index d47d9d77e6..33827136a1 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -1120,11 +1120,14 @@ project-kill-buffer-conditions (defun project--buffer-list (pr) "Return the list of all buffers in project PR." - (let (bufs) + (let ((remote-project-p (file-remote-p (project-root pr))) + bufs) (dolist (buf (buffer-list)) - (when (equal pr - (with-current-buffer buf - (project-current))) + (when (and (let ((remote (file-remote-p (buffer-local-value 'default-directory buf)))) + (if remote-project-p remote (not remote))) + (equal pr + (with-current-buffer buf + (project-current)))) (push buf bufs))) (nreverse bufs))) -- 2.30.2 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 9:43 [PATCH] Speed up project-kill-buffers Philip Kaludercic @ 2021-05-03 12:46 ` Stefan Monnier 2021-05-03 13:06 ` Philip Kaludercic 2021-05-03 21:43 ` Dmitry Gutov 1 sibling, 1 reply; 26+ messages in thread From: Stefan Monnier @ 2021-05-03 12:46 UTC (permalink / raw To: Philip Kaludercic; +Cc: emacs-devel > (defun project--buffer-list (pr) > "Return the list of all buffers in project PR." > - (let (bufs) > + (let ((remote-project-p (file-remote-p (project-root pr))) > + bufs) > (dolist (buf (buffer-list)) > - (when (equal pr > - (with-current-buffer buf > - (project-current))) > + (when (and (let ((remote (file-remote-p (buffer-local-value 'default-directory buf)))) > + (if remote-project-p remote (not remote))) > + (equal pr > + (with-current-buffer buf > + (project-current)))) > (push buf bufs))) > (nreverse bufs))) How 'bout using `file-in-directory-p`? Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 12:46 ` Stefan Monnier @ 2021-05-03 13:06 ` Philip Kaludercic 2021-05-03 13:24 ` Stefan Monnier 0 siblings, 1 reply; 26+ messages in thread From: Philip Kaludercic @ 2021-05-03 13:06 UTC (permalink / raw To: Stefan Monnier; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 1016 bytes --] Stefan Monnier <monnier@iro.umontreal.ca> writes: >> (defun project--buffer-list (pr) >> "Return the list of all buffers in project PR." >> - (let (bufs) >> + (let ((remote-project-p (file-remote-p (project-root pr))) >> + bufs) >> (dolist (buf (buffer-list)) >> - (when (equal pr >> - (with-current-buffer buf >> - (project-current))) >> + (when (and (let ((remote (file-remote-p (buffer-local-value 'default-directory buf)))) >> + (if remote-project-p remote (not remote))) >> + (equal pr >> + (with-current-buffer buf >> + (project-current)))) >> (push buf bufs))) >> (nreverse bufs))) > > How 'bout using `file-in-directory-p`? I didn't know about that function! Just tried it out and it seems that the patch below is even faster, as project-current does not have to be invoked for every buffer, remote or not. > Stefan > -- Philip K. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Reduce-number-of-method-invocations-in-project-buffe.patch --] [-- Type: text/x-diff, Size: 1136 bytes --] From eb9f32e1007eaa90a1b5487ac009b38182d6260b Mon Sep 17 00:00:00 2001 From: Philip K <philipk@posteo.net> Date: Mon, 3 May 2021 11:35:41 +0200 Subject: [PATCH] Reduce number of method invocations in project--buffer-list * project.el (project--buffer-list): Use file-in-directory-p --- lisp/progmodes/project.el | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index d47d9d77e6..aa2fc1690f 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -1120,11 +1120,12 @@ project-kill-buffer-conditions (defun project--buffer-list (pr) "Return the list of all buffers in project PR." - (let (bufs) + (let ((root (project-root pr)) + bufs) (dolist (buf (buffer-list)) - (when (equal pr - (with-current-buffer buf - (project-current))) + (when-let ((file (or (buffer-file-name buf) + (buffer-local-value 'default-directory buf))) + ((file-in-directory-p file root))) (push buf bufs))) (nreverse bufs))) -- 2.30.2 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 13:06 ` Philip Kaludercic @ 2021-05-03 13:24 ` Stefan Monnier 2021-05-03 17:25 ` Philip Kaludercic 2021-05-08 12:03 ` Stephen Leake 0 siblings, 2 replies; 26+ messages in thread From: Stefan Monnier @ 2021-05-03 13:24 UTC (permalink / raw To: Philip Kaludercic; +Cc: emacs-devel > I didn't know about that function! Just tried it out and it seems that > the patch below is even faster, as project-current does not have to be > invoked for every buffer, remote or not. I think it might misbehave if you have projects nested in the directory of other projects, tho. Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 13:24 ` Stefan Monnier @ 2021-05-03 17:25 ` Philip Kaludercic 2021-05-03 21:12 ` Dmitry Gutov 2021-05-08 12:03 ` Stephen Leake 1 sibling, 1 reply; 26+ messages in thread From: Philip Kaludercic @ 2021-05-03 17:25 UTC (permalink / raw To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> I didn't know about that function! Just tried it out and it seems that >> the patch below is even faster, as project-current does not have to be >> invoked for every buffer, remote or not. > > I think it might misbehave if you have projects nested in the directory > of other projects, tho. That is true, but this leads to the more general question of whether a sub-project is part of the super-project or not? Is there any reason to intrinsically prefer one over the other? > Stefan -- Philip K. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 17:25 ` Philip Kaludercic @ 2021-05-03 21:12 ` Dmitry Gutov 0 siblings, 0 replies; 26+ messages in thread From: Dmitry Gutov @ 2021-05-03 21:12 UTC (permalink / raw To: Philip Kaludercic, Stefan Monnier; +Cc: emacs-devel On 03.05.2021 20:25, Philip Kaludercic wrote: > That is true, but this leads to the more general question of whether a > sub-project is part of the super-project or not? Is there any reason to > intrinsically prefer one over the other? If sub-project is part of the super-project, what makes it a separate project, then? If we do consider it separate, I think it makes sense to only kill buffers belonging to it, but not the parent. And vice versa. Ultimately, it's a non-trivial question, and we'll probably delegate it to project backends as well. That will still require getting the current project. We also want to support arbitrary project-finding strategies which dosn't depend on the parent directory (see commit 4ca13d98c9eb and the referenced discussion). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 13:24 ` Stefan Monnier 2021-05-03 17:25 ` Philip Kaludercic @ 2021-05-08 12:03 ` Stephen Leake 2021-05-08 12:30 ` Philip Kaludercic 2021-05-08 17:10 ` Dmitry Gutov 1 sibling, 2 replies; 26+ messages in thread From: Stephen Leake @ 2021-05-08 12:03 UTC (permalink / raw To: Stefan Monnier; +Cc: Philip Kaludercic, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> I didn't know about that function! Just tried it out and it seems that >> the patch below is even faster, as project-current does not have to be >> invoked for every buffer, remote or not. > > I think it might misbehave if you have projects nested in the directory > of other projects, tho. There are also projects that do not have a single root; an Emacs elisp project (which has load-path as roots), any project with dependent external libraries. You can check (member file (project-files prj)), but that's about the same as your first patch. -- -- Stephe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-08 12:03 ` Stephen Leake @ 2021-05-08 12:30 ` Philip Kaludercic 2021-05-08 13:05 ` Philip Kaludercic 2021-05-08 17:01 ` Dmitry Gutov 2021-05-08 17:10 ` Dmitry Gutov 1 sibling, 2 replies; 26+ messages in thread From: Philip Kaludercic @ 2021-05-08 12:30 UTC (permalink / raw To: Stephen Leake; +Cc: Stefan Monnier, emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Stefan Monnier <monnier@iro.umontreal.ca> writes: > >>> I didn't know about that function! Just tried it out and it seems that >>> the patch below is even faster, as project-current does not have to be >>> invoked for every buffer, remote or not. >> >> I think it might misbehave if you have projects nested in the directory >> of other projects, tho. > > There are also projects that do not have a single root; an Emacs elisp > project (which has load-path as roots), any project with dependent > external libraries. > > You can check (member file (project-files prj)), but that's about the > same as your first patch. So maybe something like (let ((root (project-root proj)) (extr (project-external-roots proj))) (member file (project-files proj (cons root extr)))) ? -- Philip K. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-08 12:30 ` Philip Kaludercic @ 2021-05-08 13:05 ` Philip Kaludercic 2021-05-08 17:01 ` Dmitry Gutov 1 sibling, 0 replies; 26+ messages in thread From: Philip Kaludercic @ 2021-05-08 13:05 UTC (permalink / raw To: Stephen Leake; +Cc: Stefan Monnier, emacs-devel Philip Kaludercic <philipk@posteo.net> writes: > So maybe something like > > (let ((root (project-root proj)) > (extr (project-external-roots proj))) > (member file (project-files proj (cons root extr)))) I realize that one issue here is that external roots could be shared by multiple projects, so you wouldn't want to kill them when cleaning up after a project even if you would want to have them listed. > ? -- Philip K. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-08 12:30 ` Philip Kaludercic 2021-05-08 13:05 ` Philip Kaludercic @ 2021-05-08 17:01 ` Dmitry Gutov 1 sibling, 0 replies; 26+ messages in thread From: Dmitry Gutov @ 2021-05-08 17:01 UTC (permalink / raw To: Philip Kaludercic, Stephen Leake; +Cc: Stefan Monnier, emacs-devel On 08.05.2021 15:30, Philip Kaludercic wrote: > So maybe something like > > (let ((root (project-root proj)) > (extr (project-external-roots proj))) > (member file (project-files proj (cons root extr)))) I don't think you'll like the performance characteristics of this approach. ;-) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-08 12:03 ` Stephen Leake 2021-05-08 12:30 ` Philip Kaludercic @ 2021-05-08 17:10 ` Dmitry Gutov 2021-05-16 20:36 ` Stephen Leake 1 sibling, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-08 17:10 UTC (permalink / raw To: Stephen Leake, Stefan Monnier; +Cc: Philip Kaludercic, emacs-devel Hi Stephen, On 08.05.2021 15:03, Stephen Leake wrote: > There are also projects that do not have a single root; an Emacs elisp > project (which has load-path as roots), any project with dependent > external libraries. This is not really than different. The VC backend has load-path in external roots (which can have counterparts in "dependency libraries" in other languages). When we close such project, though, we usually don't want to kill any of the buffers belonging to external libraries (they might as well be part of some other project, e.g. one that the current depends on). To support arbitrarily-shaped projects, though, we can introduce a method like 'project-contains-buffer?'. Which you would implement with 'member'. The performance could be okay if your projects are small enough. It doesn't handle non-file-visiting buffers, though. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-08 17:10 ` Dmitry Gutov @ 2021-05-16 20:36 ` Stephen Leake 2021-05-16 21:58 ` Dmitry Gutov 0 siblings, 1 reply; 26+ messages in thread From: Stephen Leake @ 2021-05-16 20:36 UTC (permalink / raw To: Dmitry Gutov; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > When we close such project, though, we usually ^^^ Which means you need to provide an option for the times when we want the other behavior: C-u kills root and external-roots, plain kills root only. -- -- Stephe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-16 20:36 ` Stephen Leake @ 2021-05-16 21:58 ` Dmitry Gutov 2021-05-19 23:37 ` Stephen Leake 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-16 21:58 UTC (permalink / raw To: Stephen Leake; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel On 16.05.2021 23:36, Stephen Leake wrote: > Which means you need to provide an option for the times when we want the > other behavior: C-u kills root and external-roots, plain kills root only. I wouldn't mind adding an option, but that presents a difficulty. At the moment, we call `project-current` in each buffer and compare the returned value to the "current" project instance. That wouldn't work for external roots no matter what option we add because a given file inside "external root" can belong (in an "extended" sense) to several projects at once, so there's no logical way to obtain the project instance we're looking for based on such external file. Right? A generic like project-contains? I previously mentioned would have a problem that every project backend's implementation would have to obey such an option, which creates new possibility for bugs and diverging behaviors. If the current project-kill-buffers doesn't work for you (please check; IIRC your backend's peculiar behavior might have an upside in this case), what we could more easily do is create an option like 'project-list-buffers-function'. The default would delegate to project-current. The alternative could indeed use file-in-directory-p (or a string comparison to similar effect) as discussed in this thread, providing a different inclusion semantics and improved performance, and the third alternative value could enable looking in external roots as well. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-16 21:58 ` Dmitry Gutov @ 2021-05-19 23:37 ` Stephen Leake 2021-05-20 12:16 ` Stephen Leake 2021-05-25 1:24 ` Dmitry Gutov 0 siblings, 2 replies; 26+ messages in thread From: Stephen Leake @ 2021-05-19 23:37 UTC (permalink / raw To: Dmitry Gutov; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > On 16.05.2021 23:36, Stephen Leake wrote: >> Which means you need to provide an option for the times when we want the >> other behavior: C-u kills root and external-roots, plain kills root only. > > I wouldn't mind adding an option, but that presents a difficulty. > > At the moment, we call `project-current` in each buffer and compare > the returned value to the "current" project instance. That wouldn't > work for external roots no matter what option we add because a given > file inside "external root" can belong (in an "extended" sense) to > several projects at once, so there's no logical way to obtain the > project instance we're looking for based on such external file. Right? There is in my projects; the currently active global project. Which means that any buffer will appear to "belong" to that project, unless project-find-functions is buffer-local, as it is in my elisp buffers. I view project-current as returning an object useful for project-related commands. So in a "notes" text file, where I keep track of things to do for a project, I want project-find-file to use that project (which is the current globally selected project). project-delete-buffers should delete that notes file. But in a higher level notes file, opened on Emacs start, that lists all the projects I'm currently working on, project-find-file should also use the current global project, but that buffer should not be deleted with the project. > A generic like project-contains? That's one option, with the current predicate in project--buffer-list as the default implementation; then I could override that to check project-files, or all roots, or something. As long as it passes C-u to the backend, my function could also use that to decide about dependent libraries. It might be better to make the predicate more specific; project-delete-this-buffer-p. That way eglot won't try to abuse project-contains to pick a server :). Or maybe my delete buffer case and eglot's "choose the right server" case really are the same? > I previously mentioned would have a problem that every project > backend's implementation would have to obey such an option, which > creates new possibility for bugs and diverging behaviors. Yes. Things should be as simple as possible, but no simpler. A default that is useful in many simple projects helps a lot. > If the current project-kill-buffers doesn't work for you (please > check; IIRC your backend's peculiar behavior might have an upside in > this case), First, M-x project-kill-buffers dies because my definition of project-root returns nil. I suppose you'd call that a bug in my project backend, so I'll pick some arbitrary directory (essentially (car compilation-search-path)) and call it the "primary root". Then the prompt is: Kill 17 buffers in d:/apps/gnat-gpl_2020/lib/gnat? (yes or no) That directory is something you would call an external root, I guess, so you would argue I should pick a different one. But I have lived with project-root returning nil for several years now, so I don't see why I should be forced to put effort into choosing some "better" root, just so project-kill-buffers can detect a remote project. I'd be happy if that remote project predicate treated nil as "not remote". In any case, 17 buffers is far too many; there are only three buffers open related to the current project. (length (buffer-list)) is 24, so at least it's not deleting everything. Hmm. I just found project-kill-buffer-conditions; maybe I could customize that; it allows arbitrary predicate functions. It would be more efficient if the predicate function were also passed the project object, so it doesn't have to call project-current again (hmm - shades of project-delete-this-buffer-p :). A better prompt would be the name of the project; the ELPA package wisi defines a name for each project, specified by the user as part of the project definition. In this case, it is "ada_mode_wisi_parse stephe_6", which is much more meaningfull to me, and more meaningfull than some "better" "primary root" directory. In fact, I have two different projects that share the directory of test files (one for compiling the parser, one for running the parser tests), so a directory name does not uniquely identify a project. Similarly, there are elisp and Ada files in the ada-mode source directory; the elisp files use the elisp project (named "elisp"), the Ada (and all other) files use the "ada_mode_wisi_parse stephe_6" project. I think another generic project-name, or possbly project-prompt, would be good here. The default could be project-root. There should also be a custom boolean to turn off the prompt; it's helpful the first few times, then it's just annoying. Currently C-u is 'no-confirm'; I hope that will change to 'no-dependencies'. Normal Emacs style uses a custom variable for suppressing prompts. Even better would to define the key ? at that prompt to put up a list of the buffer names; that way the user could learn to trust the command (I certainly don't trust it now :). I suspect that would be very hard to do (certainly yes-or-no-p can't do that now). Instead, the custom variable that turns off the prompt could be tri-valued; nil (no prompt), t (short prompt), 'verbose (list all buffers, with a hint on how to change the prompt behavior). Then 'verbose should be the default. dired has similar behavior when deleting one vs several files. > what we could more easily do is create an option like > 'project-list-buffers-function'. The default would delegate to > project-current. I don't see how that is better/easier than a cl-defgeneric; either way the project has to specify the correct function, possibly on a per-project basis. Either way you have to provide a useful default implementation. And they provide the same power to encounter/create bugs; they are both dispatching methods. I think it's better to stick to one method of dispatching. In my case, the wisi package will provide the function. On the other hand, it appears we already have dispatching via project-kill-buffer-conditions; I'll see if I can make that work. I'm not sure I can reliably detect C-u from there for dependent libraries. I already have a function similar to 'project-kill-buffers', defined in my vc package; it kills buffers whose files are under a root directory. That solves the question of whether to delete buffers in dependent libraries, at the expense of requiring the user to invoke it several times in order to fully clean up. So a correct solution for project-kill-buffers, with a simple invocation-time option for dependent libraries, would be nice. -- -- Stephe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-19 23:37 ` Stephen Leake @ 2021-05-20 12:16 ` Stephen Leake 2021-05-25 1:30 ` Dmitry Gutov 2021-05-25 1:24 ` Dmitry Gutov 1 sibling, 1 reply; 26+ messages in thread From: Stephen Leake @ 2021-05-20 12:16 UTC (permalink / raw To: Dmitry Gutov; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: > Hmm. I just found project-kill-buffer-conditions; maybe I could > customize that; it allows arbitrary predicate functions. It would be > more efficient if the predicate function were also passed the project > object, so it doesn't have to call project-current again (hmm - shades > of project-delete-this-buffer-p :). This works for wisi projects, but not for the elisp project (it signals an error): (defun wisi-prj-kill-buffer-condition (buffer) "Return non-nil if BUFFER should be killed. For `project-kill-buffer-conditions'." (let* ((source-path (wisi-prj-source-path (project-current))) (buf-file-name (buffer-file-name buffer)) (done (not (buffer-file-name buffer))) (result nil) dir) (while (and source-path (not done)) (setq dir (pop source-path)) (when (and dir (file-in-directory-p buf-file-name dir)) (setq done t) (setq result t))) result)) (setq project-kill-buffer-conditions (list #'wisi-prj-kill-buffer-condition)) I'm not clear how to set project-kill-buffer-conditions on a per-project basis; it would be simple if this was a cl-defgeneric dispatching on the project. -- -- Stephe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-20 12:16 ` Stephen Leake @ 2021-05-25 1:30 ` Dmitry Gutov 2021-05-30 17:19 ` Stephen Leake 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-25 1:30 UTC (permalink / raw To: Stephen Leake; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel On 20.05.2021 15:16, Stephen Leake wrote: > Stephen Leake <stephen_leake@stephe-leake.org> writes: > >> Hmm. I just found project-kill-buffer-conditions; maybe I could >> customize that; it allows arbitrary predicate functions. It would be >> more efficient if the predicate function were also passed the project >> object, so it doesn't have to call project-current again (hmm - shades >> of project-delete-this-buffer-p :). > > This works for wisi projects, but not for the elisp project (it signals > an error): I suppose if you set project-find-functions to a local value in emacs-lisp-mode buffers (which is IMO unusual), you could also set project-kill-buffer-conditions to a different local value in those buffers? > (defun wisi-prj-kill-buffer-condition (buffer) > "Return non-nil if BUFFER should be killed. > For `project-kill-buffer-conditions'." > (let* ((source-path (wisi-prj-source-path (project-current))) Are those just project sources or dependency dirs as well? > (buf-file-name (buffer-file-name buffer)) > (done (not (buffer-file-name buffer))) > (result nil) > dir) > (while (and source-path > (not done)) > (setq dir (pop source-path)) > (when (and dir > (file-in-directory-p buf-file-name dir)) > (setq done t) > (setq result t))) > result)) On the face of it, project-kill-buffer-conditions could use a PROJECT argument to improve efficiency, but if you have a var which points to the global project, your 'project-current' probably works very quickly. So there wouldn't be much actual speedup in your case. You can prove me wrong with (benchmark 1 '...) or (benchmark-progn ...). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-25 1:30 ` Dmitry Gutov @ 2021-05-30 17:19 ` Stephen Leake 0 siblings, 0 replies; 26+ messages in thread From: Stephen Leake @ 2021-05-30 17:19 UTC (permalink / raw To: emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: > On 20.05.2021 15:16, Stephen Leake wrote: >> Stephen Leake <stephen_leake@stephe-leake.org> writes: >> >>> Hmm. I just found project-kill-buffer-conditions; maybe I could >>> customize that; it allows arbitrary predicate functions. It would be >>> more efficient if the predicate function were also passed the project >>> object, so it doesn't have to call project-current again (hmm - shades >>> of project-delete-this-buffer-p :). >> This works for wisi projects, but not for the elisp project (it >> signals >> an error): > > I suppose if you set project-find-functions to a local value in > emacs-lisp-mode buffers (which is IMO unusual), you could also set > project-kill-buffer-conditions to a different local value in those > buffers? yes, that would work. >> (defun wisi-prj-kill-buffer-condition (buffer) >> "Return non-nil if BUFFER should be killed. >> For `project-kill-buffer-conditions'." >> (let* ((source-path (wisi-prj-source-path (project-current))) > > Are those just project sources or dependency dirs as well? Both; it has never been useful to try to make a distinction. -- -- Stephe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-19 23:37 ` Stephen Leake 2021-05-20 12:16 ` Stephen Leake @ 2021-05-25 1:24 ` Dmitry Gutov 2021-05-30 17:51 ` Stephen Leake 1 sibling, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-25 1:24 UTC (permalink / raw To: Stephen Leake; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel On 20.05.2021 02:37, Stephen Leake wrote: >> At the moment, we call `project-current` in each buffer and compare >> the returned value to the "current" project instance. That wouldn't >> work for external roots no matter what option we add because a given >> file inside "external root" can belong (in an "extended" sense) to >> several projects at once, so there's no logical way to obtain the >> project instance we're looking for based on such external file. Right? > > There is in my projects; the currently active global project. Which > means that any buffer will appear to "belong" to that project, unless > project-find-functions is buffer-local, as it is in my elisp buffers. Oh. Right. So simply comparing project-current won't work for "global" projects like in your setup. > I view project-current as returning an object useful for project-related > commands. So in a "notes" text file, where I keep track of things to do > for a project, I want project-find-file to use that project (which is > the current globally selected project). project-delete-buffers should > delete that notes file. But in a higher level notes file, opened on > Emacs start, that lists all the projects I'm currently working on, > project-find-file should also use the current global project, but that > buffer should not be deleted with the project. Right. >> A generic like project-contains? > > That's one option, with the current predicate in project--buffer-list as > the default implementation; then I could override that to check > project-files, or all roots, or something. As long as it passes C-u to > the backend, my function could also use that to decide about dependent > libraries. I'm fairly sure you don't want to close the buffers visiting the dependencies. Or if we do, that would be a globally acting modifier, and that piece of logic which we would add to project-kill-buffers would just see whether the buffer's default-directory is inside any of the "external roots", as defined by the backend. Would that work for you? > It might be better to make the predicate more specific; > project-delete-this-buffer-p. That way eglot won't try to abuse > project-contains to pick a server :). Or maybe my delete buffer case > and eglot's "choose the right server" case really are the same? I'd rather we try to be more strict with semantics, if possible, and 'project-contains?' would only return t for buffers "inside" the project, not stuff that's outside but related to it (like external libraries, system includes, etc, which is what I'd like "external roots" to be targeted at). >> I previously mentioned would have a problem that every project >> backend's implementation would have to obey such an option, which >> creates new possibility for bugs and diverging behaviors. > > Yes. Things should be as simple as possible, but no simpler. > > A default that is useful in many simple projects helps a lot. It also helps when we can encourage projects not to forget to obey the option, some way or other. >> If the current project-kill-buffers doesn't work for you (please >> check; IIRC your backend's peculiar behavior might have an upside in >> this case), > > First, M-x project-kill-buffers dies because my definition of > project-root returns nil. I suppose you'd call that a bug in my project > backend, so I'll pick some arbitrary directory (essentially (car > compilation-search-path)) and call it the "primary root". > > Then the prompt is: > > Kill 17 buffers in d:/apps/gnat-gpl_2020/lib/gnat? (yes or no) > > That directory is something you would call an external root, I guess, so > you would argue I should pick a different one. But I have lived with > project-root returning nil for several years now, so I don't see why I > should be forced to put effort into choosing some "better" root, just so > project-kill-buffers can detect a remote project. I'd be happy if that > remote project predicate treated nil as "not remote". Even if your project is "amorphous" in shape, I wouldn't necessarily want to call its directories "external roots". For project-root, though, you might pick something like the directory that hosts the main configuration/build file. That wouldn't help with the project-list-buffers-function approach, but I suppose that just wasn't a great idea. > In any case, 17 buffers is far too many; there are only three buffers > open related to the current project. (length (buffer-list)) is 24, so at > least it's not deleting everything. > > Hmm. I just found project-kill-buffer-conditions; maybe I could > customize that; it allows arbitrary predicate functions. It would be > more efficient if the predicate function were also passed the project > object, so it doesn't have to call project-current again (hmm - shades > of project-delete-this-buffer-p :). Yes, it's an interesting idea: if all buffers point to the current project, you could do post-filtering in this var. > A better prompt would be the name of the project; the ELPA package wisi > defines a name for each project, specified by the user as part of the > project definition. In this case, it is "ada_mode_wisi_parse stephe_6", > which is much more meaningfull to me, and more meaningfull than some > "better" "primary root" directory. In fact, I have two different > projects that share the directory of test files (one for compiling the > parser, one for running the parser tests), so a directory name does not > uniquely identify a project. Similarly, there are elisp and Ada files in > the ada-mode source directory; the elisp files use the elisp project > (named "elisp"), the Ada (and all other) files use the > "ada_mode_wisi_parse stephe_6" project. > > I think another generic project-name, or possbly project-prompt, would > be good here. The default could be project-root. We can add that, but please file that as a separate bug report. > There should also be a custom boolean to turn off the prompt; it's > helpful the first few times, then it's just annoying. Currently C-u is > 'no-confirm'; I hope that will change to 'no-dependencies'. It *might* change to "include dependencies", since currently it's not supposed to include them. If you want this change to happen, could you outline the cases when you would and would not use this capability? Personally, I'd probably never kill those buffers. > Normal Emacs > style uses a custom variable for suppressing prompts. See below. > Even better would to define the key ? at that prompt to put up a list of > the buffer names; that way the user could learn to trust the command (I > certainly don't trust it now :). I suspect that would be very hard to do > (certainly yes-or-no-p can't do that now). That would require some work (patches welcome?), but shouldn't be too difficult > Instead, the custom variable > that turns off the prompt could be tri-valued; nil (no prompt), t (short > prompt), 'verbose (list all buffers, with a hint on how to change the > prompt behavior). Then 'verbose should be the default. dired has similar > behavior when deleting one vs several files. I'd be happy to review a patch. If there are many such buffers, some smart formatting of the list would need to be used, though. >> what we could more easily do is create an option like >> 'project-list-buffers-function'. The default would delegate to >> project-current. > > I don't see how that is better/easier than a cl-defgeneric; either way > the project has to specify the correct function, possibly on a > per-project basis. Either way you have to provide a useful default > implementation. And they provide the same power to encounter/create > bugs; they are both dispatching methods. I think it's better to stick to > one method of dispatching. In my case, the wisi package will provide the > function. Consider also this: you suggested a generic like project-delete-this-buffer-p. If in the end the set of buffers that project-switch-to-buffer uses, and the one that project-kill-buffer uses, are very different, perhaps there is not much benefit in having both of them do this through the project API. Because the point of doing that is to be able to reuse the same information in different contexts. Like, if your backend would have to implement a method that's 5x as long as the current project-kill-buffers definition, you might as well provide an ada-project-kill-buffers command, right? Just thinking aloud here. > On the other hand, it appears we already have dispatching via > project-kill-buffer-conditions; I'll see if I can make that work. I'm > not sure I can reliably detect C-u from there for dependent libraries. Let me take a look at your other email. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-25 1:24 ` Dmitry Gutov @ 2021-05-30 17:51 ` Stephen Leake 2021-08-09 3:01 ` Dmitry Gutov 0 siblings, 1 reply; 26+ messages in thread From: Stephen Leake @ 2021-05-30 17:51 UTC (permalink / raw To: Dmitry Gutov; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel Dmitry Gutov <dgutov@yandex.ru> writes: >>> A generic like project-contains? >> That's one option, with the current predicate in >> project--buffer-list as >> the default implementation; then I could override that to check >> project-files, or all roots, or something. As long as it passes C-u to >> the backend, my function could also use that to decide about dependent >> libraries. > > I'm fairly sure you don't want to close the buffers visiting the > dependencies. Sometimes I do; I'm done working on ada-mode, and I transition to working on my music database, closing all ada-mode related files. Other times I don't; I found a bug in ada-mode, so I move to the wisitoken library to run tests there, closing only the higher-level ada-mode files. The point is that it is up to the user, to decide on a case by case basis. > Or if we do, that would be a globally acting modifier, and that piece > of logic which we would add to project-kill-buffers would just see > whether the buffer's default-directory is inside any of the "external > roots", as defined by the backend. Would that work for you? I don't follow. What is the UI? How does the information given by the user get passed to the backend? Only the backend can interpret "dependencies". And no, not all project files can be distinguished by directory; I have files in the same directory that are managed by different projects (Ada and elisp). >> It might be better to make the predicate more specific; >> project-delete-this-buffer-p. That way eglot won't try to abuse >> project-contains to pick a server :). Or maybe my delete buffer case >> and eglot's "choose the right server" case really are the same? > > I'd rather we try to be more strict with semantics, if possible, and > 'project-contains?' would only return t for buffers "inside" the > project, not stuff that's outside but related to it (like external > libraries, system includes, etc, which is what I'd like "external > roots" to be targeted at). Then you need an "include-external-roots" arg to project-contains, since sometimes that's what the user wants. >> I think another generic project-name, or possbly project-prompt, >> would >> be good here. The default could be project-root. > > We can add that, but please file that as a separate bug report. Done; 48747 >> There should also be a custom boolean to turn off the prompt; it's >> helpful the first few times, then it's just annoying. Currently C-u is >> 'no-confirm'; I hope that will change to 'no-dependencies'. > > It *might* change to "include dependencies", since currently it's not > supposed to include them. > > If you want this change to happen, could you outline the cases when > you would and would not use this capability? Personally, I'd probably > never kill those buffers. I gave examples above. In general, if I'm switching to a project that shares some files with the current one, I don't want to delete those buffers. So the correct semantics would be "switching from project A to project B; close all non-shared buffers". -- -- Stephe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-30 17:51 ` Stephen Leake @ 2021-08-09 3:01 ` Dmitry Gutov 0 siblings, 0 replies; 26+ messages in thread From: Dmitry Gutov @ 2021-08-09 3:01 UTC (permalink / raw To: Stephen Leake; +Cc: Philip Kaludercic, Stefan Monnier, emacs-devel Hi Stephen, I've posted a patch to https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49264#38 which should add the necessary flexibility, for your project backend to be able to list its buffers without being tied to the root dir. So far it just adds a generic called 'project-buffers'. On 30.05.2021 20:51, Stephen Leake wrote: > Dmitry Gutov <dgutov@yandex.ru> writes: >> I'm fairly sure you don't want to close the buffers visiting the >> dependencies. > > Sometimes I do; I'm done working on ada-mode, and I transition to > working on my music database, closing all ada-mode related files. Other > times I don't; I found a bug in ada-mode, so I move to the wisitoken > library to run tests there, closing only the higher-level ada-mode > files. > > The point is that it is up to the user, to decide on a case by case > basis. If the distinction is indeed useful, maybe the change mentioned above can be followed with generic 'project-externals-buffers'. Either way, it seems you might prefer a new command like 'project-kill-other-buffers' (as opposed to 'project-kill-buffers'), which would kill all buffers (which satisfy project-kill-buffer-conditions, of course) that don't belong to the current project. >> Or if we do, that would be a globally acting modifier, and that piece >> of logic which we would add to project-kill-buffers would just see >> whether the buffer's default-directory is inside any of the "external >> roots", as defined by the backend. Would that work for you? > > I don't follow. What is the UI? How does the information given by the > user get passed to the backend? Only the backend can interpret > "dependencies". Yeah, scratch that. >> I'd rather we try to be more strict with semantics, if possible, and >> 'project-contains?' would only return t for buffers "inside" the >> project, not stuff that's outside but related to it (like external >> libraries, system includes, etc, which is what I'd like "external >> roots" to be targeted at). > > Then you need an "include-external-roots" arg to project-contains, since > sometimes that's what the user wants. For now it's project-buffers and (potentially) project-externals-buffers, for nicer naming and being able to implement in the vc backend in a performant fashion more easily (having predicate methods would require caching some information in the project instance between the calls). But project-contains-buffer-p and project-externals-contain-buffer-p are still on the table, if anybody feels strongly about that choice. >> If you want this change to happen, could you outline the cases when >> you would and would not use this capability? Personally, I'd probably >> never kill those buffers. > > I gave examples above. In general, if I'm switching to a project that > shares some files with the current one, I don't want to delete those > buffers. So the correct semantics would be "switching from project A to > project B; close all non-shared buffers". Sounds like 'project-kill-other-buffers' I described above. You might not even need to make a distinction between project contents and "externals" for this scenario (having 'project-buffers' return all pertinent buffers). But there can be other scenarios, I guess. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 9:43 [PATCH] Speed up project-kill-buffers Philip Kaludercic 2021-05-03 12:46 ` Stefan Monnier @ 2021-05-03 21:43 ` Dmitry Gutov 2021-05-03 22:45 ` Stefan Monnier 1 sibling, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-03 21:43 UTC (permalink / raw To: Philip Kaludercic, emacs-devel Hi Philip, On 03.05.2021 12:43, Philip Kaludercic wrote: > I've noticed that sometimes project-kill-buffers is noticeably slow, and > it seems like it's has to do with project--buffer-list working on remote > files. The function goes through every buffer and calls > (project-current), even if the buffer is related to a remote file that > cannot be part of the current project. > > The patch I attach below is a simple fix to avoid checking files that > cannot be part of the current project. Or are there any edge-cases that > this code approach breaks? In theory, files on different hosts could be part of the same "project" (in Eli's sense, see https://lists.gnu.org/archive/html/emacs-devel/2020-07/msg00051.html and the other messages in that thread), but we'll get there when we get there. Probably by adding a method like project-contains-file-p. In the meantime (until somebody complains), the patch like this should be fine. I haven't found any significant difference in performance, but I don't have Tramp buffers in the current session. Pushed to master, thanks. If we do believe that a project can only span one host, we could also change the check like this: diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index 33827136a1..6f911e4fbe 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -1120,11 +1120,11 @@ project-kill-buffer-conditions (defun project--buffer-list (pr) "Return the list of all buffers in project PR." - (let ((remote-project-p (file-remote-p (project-root pr))) + (let ((conn (file-remote-p (project-root pr))) bufs) (dolist (buf (buffer-list)) - (when (and (let ((remote (file-remote-p (buffer-local-value 'default-directory buf)))) - (if remote-project-p remote (not remote))) + (when (and (equal conn + (file-remote-p (buffer-local-value 'default-directory buf))) (equal pr (with-current-buffer buf (project-current)))) WDYT? ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 21:43 ` Dmitry Gutov @ 2021-05-03 22:45 ` Stefan Monnier 2021-05-03 22:46 ` Dmitry Gutov 0 siblings, 1 reply; 26+ messages in thread From: Stefan Monnier @ 2021-05-03 22:45 UTC (permalink / raw To: Dmitry Gutov; +Cc: Philip Kaludercic, emacs-devel >> I've noticed that sometimes project-kill-buffers is noticeably slow, and >> it seems like it's has to do with project--buffer-list working on remote >> files. The function goes through every buffer and calls >> (project-current), even if the buffer is related to a remote file that >> cannot be part of the current project. >> The patch I attach below is a simple fix to avoid checking files that >> cannot be part of the current project. Or are there any edge-cases that >> this code approach breaks? > > In theory, files on different hosts could be part of the same "project" (in > Eli's sense, see > https://lists.gnu.org/archive/html/emacs-devel/2020-07/msg00051.html and the > other messages in that thread), but we'll get there when we get > there. Probably by adding a method like project-contains-file-p. Of course, we could also make the check super fast by keeping the "current project" in a buffer-local var, and then just look for buffers which have an `eq` value in that var. That would be both faster than `file-in-directory-p` and more arguably more correct. Stefan ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 22:45 ` Stefan Monnier @ 2021-05-03 22:46 ` Dmitry Gutov 2021-05-04 6:25 ` tomas 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-03 22:46 UTC (permalink / raw To: Stefan Monnier; +Cc: Philip Kaludercic, emacs-devel On 04.05.2021 01:45, Stefan Monnier wrote: > Of course, we could also make the check super fast by keeping the > "current project" in a buffer-local var, and then just look for buffers > which have an `eq` value in that var. That would be both faster than > `file-in-directory-p` and more arguably more correct. Until the user changes some project setting and the "current project" becomes not current anymore. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-03 22:46 ` Dmitry Gutov @ 2021-05-04 6:25 ` tomas 2021-05-04 10:40 ` Dmitry Gutov 0 siblings, 1 reply; 26+ messages in thread From: tomas @ 2021-05-04 6:25 UTC (permalink / raw To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 686 bytes --] On Tue, May 04, 2021 at 01:46:43AM +0300, Dmitry Gutov wrote: > On 04.05.2021 01:45, Stefan Monnier wrote: > >Of course, we could also make the check super fast by keeping the > >"current project" in a buffer-local var, and then just look for buffers > >which have an `eq` value in that var. That would be both faster than > >`file-in-directory-p` and more arguably more correct. > > Until the user changes some project setting and the "current > project" becomes not current anymore. All that looks to me like a classical caching problem, with cache invalidation protocols and things. (Don't take me too seriously, though. I've no skin in this game). Cheers - t [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-04 6:25 ` tomas @ 2021-05-04 10:40 ` Dmitry Gutov 2021-05-04 11:26 ` tomas 0 siblings, 1 reply; 26+ messages in thread From: Dmitry Gutov @ 2021-05-04 10:40 UTC (permalink / raw To: tomas, emacs-devel On 04.05.2021 09:25, tomas@tuxteam.de wrote: > All that looks to me like a classical caching problem, with cache > invalidation protocols and things. Sure. I'm just not in a hurry to add the cache if it limits possible applications, since the current performance seems adequate most of the time. Better explore the applications first. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] Speed up project-kill-buffers 2021-05-04 10:40 ` Dmitry Gutov @ 2021-05-04 11:26 ` tomas 0 siblings, 0 replies; 26+ messages in thread From: tomas @ 2021-05-04 11:26 UTC (permalink / raw To: Dmitry Gutov; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 412 bytes --] On Tue, May 04, 2021 at 01:40:18PM +0300, Dmitry Gutov wrote: > On 04.05.2021 09:25, tomas@tuxteam.de wrote: > >All that looks to me like a classical caching problem [...] > Sure. I'm just not in a hurry to add the cache if it limits possible > applications, since the current performance seems adequate most of > the time. > > Better explore the applications first. Makes sense, thanks :) - t [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-08-09 3:01 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-05-03 9:43 [PATCH] Speed up project-kill-buffers Philip Kaludercic 2021-05-03 12:46 ` Stefan Monnier 2021-05-03 13:06 ` Philip Kaludercic 2021-05-03 13:24 ` Stefan Monnier 2021-05-03 17:25 ` Philip Kaludercic 2021-05-03 21:12 ` Dmitry Gutov 2021-05-08 12:03 ` Stephen Leake 2021-05-08 12:30 ` Philip Kaludercic 2021-05-08 13:05 ` Philip Kaludercic 2021-05-08 17:01 ` Dmitry Gutov 2021-05-08 17:10 ` Dmitry Gutov 2021-05-16 20:36 ` Stephen Leake 2021-05-16 21:58 ` Dmitry Gutov 2021-05-19 23:37 ` Stephen Leake 2021-05-20 12:16 ` Stephen Leake 2021-05-25 1:30 ` Dmitry Gutov 2021-05-30 17:19 ` Stephen Leake 2021-05-25 1:24 ` Dmitry Gutov 2021-05-30 17:51 ` Stephen Leake 2021-08-09 3:01 ` Dmitry Gutov 2021-05-03 21:43 ` Dmitry Gutov 2021-05-03 22:45 ` Stefan Monnier 2021-05-03 22:46 ` Dmitry Gutov 2021-05-04 6:25 ` tomas 2021-05-04 10:40 ` Dmitry Gutov 2021-05-04 11:26 ` tomas
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).