`consult-buffer` slowness when using `consult-org-roam` about consult-org-roam HOT 6 CLOSED

Thank you for opening this issue, @aavanian.

I corrected and clarified the comment in commit 4f5e77d.
Your use case seems to be extreme, but the search for duplicates is indeed inefficient.
Actually, there should only be very few duplicate buffers in the list returned by org-roam-buffer-list so looking for buffers with prefix CAPTURE-* should be computationally more efficient.
If you want to come up with an improved solution, it would be greatly appreciated.

Best regards,
jgru

from consult-org-roam.

Hi @aavanian,

I experimented with just removing the buffers that are prefixed with CAPTURE-.

Would you please test the following snippet whether it satisfies your performance requirements?

(defun consult-org-roam--remove-capture-dups (buffer-list)
  "Remove CAPTURE-duplicates from BUFFER-LIST."
  (let ((out-list buffer-list))
  (dolist (x buffer-list)
    (when (s-starts-with-p "CAPTURE-" (buffer-name x))
      (setq out-list (delete x out-list))
      ))
    out-list))

Best regards,
jgru

from consult-org-roam.

Thanks for the quick turn-around, however it only improve things marginally. I now realise that it's being called many times (for each candidate via the annotation lambda (defined in org-roam-buffer-source).

I'm thinking the variable org-roam-buffer-open-buffer-list should be set when/during consult-org-roam-buffer--get-roam-bufs is called by the consult mechanics.

I can read-ish the code but don't know much about emacs (and consult in particular) but here how it could look like (including your change for consult-org-roam--remove-capture-dups):

diff --git a/consult-org-roam-buffer.el b/consult-org-roam-buffer.el
index 54373a8..089c07d 100644
--- a/consult-org-roam-buffer.el
+++ b/consult-org-roam-buffer.el
@@ -95,12 +95,11 @@
 (defun consult-org-roam--remove-capture-dups (buffer-list)
   "Remove CAPTURE-duplicates from BUFFER-LIST."
   (let ((out-list buffer-list))
-  (dolist (x buffer-list)
-    (dolist (y buffer-list)
-    (when (not (eq (buffer-name x) (buffer-name y)))
-      (if (string-match-p (regexp-quote (buffer-name y)) (buffer-name x))
-        (setq out-list (delete y buffer-list))))))
-    buffer-list))
+    (dolist (x buffer-list)
+      (when (s-starts-with-p "CAPTURE-" (buffer-name x))
+        (setq out-list (delete x buffer-list))
+        ))
+    out-list))
 
 (defun consult-org-roam--buffer-list-without-dups ()
   "Return a list of all org-roam-buffers without duplicates.
@@ -110,28 +109,27 @@ sufficient to simply is org-roam-buffer-p caused by capture
 process"
   (consult-org-roam--remove-capture-dups (org-roam-buffer-list)))
 
-(defun consult-org-roam-buffer--update-open-buffer-list ()
-  "Generate an alist of the form `(TITLE . BUF)’.
+(defun consult-org-roam-buffer--update-open-buffer-list (buffer-list)
+  "Generate an alist of the form `(TITLE . BUF) from BUFFER-LIST’.
 Generate an alist of the form `(TITLE . BUF)’ where TITLE is the
 title of an open org-roam buffer."
   (setq org-roam-buffer-open-buffer-list
-    (mapcar #'consult-org-roam-buffer--add-title
-      (consult-org-roam--buffer-list-without-dups))))
+    (mapcar #'consult-org-roam-buffer--add-title buffer-list)))
 
 (defun consult-org-roam-buffer--with-title (title)
   "Find buffer name with TITLE from among the list of open org-roam buffers."
-  (consult-org-roam-buffer--update-open-buffer-list)
   (cdr (assoc title org-roam-buffer-open-buffer-list)))
 
 (defun consult-org-roam-buffer--get-roam-bufs ()
   "Return list of currently open org-roam buffers."
-  (consult--buffer-query
+  (let ((buffer-list-no-dups (consult-org-roam--buffer-list-without-dups)))
+    (consult-org-roam-buffer--update-open-buffer-list buffer-list-no-dups)
+    (consult--buffer-query
     :sort 'visibility
     :as #'consult-org-roam-buffer--get-title
     :filter t
     :predicate (lambda (buf)
-                 (member buf
-                   (consult-org-roam--buffer-list-without-dups)))))
+                   (member buf buffer-list-no-dups)))))
 
 ;; Define source for consult-buffer
 (defvar org-roam-buffer-source

It's still slightly slower than I think it should but it's definitely usable for me now (I'll try to look further).

2 related points:

• shouldn't (setq out-list (delete x buffer-list)) rather read (setq out-list (delete x out-list)) (otherwise you lose the previous deletes)?
• shouldn't disabling consult-org-roam-mode remove org-roam-buffer-source from consult-buffer-sources?

from consult-org-roam.

Hi @aavanian,

I'm thinking the variable org-roam-buffer-open-buffer-list should be
set when/during consult-org-roam-buffer--get-roam-bufs is called by
the consult mechanics.

Indeed, thank you very much for this suggestion and drafting the code.
I reviewed and tested your patch, which looks good. I merged it in commit 5d1e4a7.

It's still slightly slower than I think it should but it's definitely
usable for me now (I'll try to look further).

I would be very happy if you would track this down even further and submit a PR (or a description of how to improve it even further). I rarely open more than 10 `org-roam``-buffers at once, so I did and do not experience any slowness.

Since you think that the current implementation is usable for you as well now, do you think that this issue can/should be closed?

• shouldn't (setq out-list (delete x buffer-list)) rather read
  (setq out-list (delete x out-list)) (otherwise you lose the
  previous deletes)?

Definitely, I drafted this in a hurry, now it is corrected.

• shouldn't disabling consult-org-roam-mode remove
  org-roam-buffer-source from consult-buffer-sources?

It should be removed, I think, and consider this to be a bug.
I track this as #21. Thank you so much for pointing this out.

Best regards,
jgru

from consult-org-roam.

Thanks, closing and will revert if I find something more.

from consult-org-roam.

Thank you for closing the issue, @aavanian.
I just want to let you know that I added the removal of the org-roam-buffer-source from consult's consult-buffer-sources when disabling the minor mode in commit 96c95e5. #21 is closed now.
I hope you enjoy using consult-org-roam.

Best regards,
jgru

from consult-org-roam.