`consult-buffer` slowness when using `consult-org-roam`

[aavanian]

I've been experimenting with consult-org-roam but find that consult-buffer becomes very slow once it's activated.

profiler result w/o `consult-org-roam`

``` 152 41% + # 45 12% + file-truename 33 8% + # 29 7% + # 24 6% + read-from-minibuffer 22 5% Automatic GC 10 2% + # 10 2% + file-relative-name 7 1% + which-key--propertize-description 5 1% + mapcar 4 1% + marginalia--function-doc 2 0% + vertico--update 2 0% + # 2 0% + all-completions 1 0% + # 1 0% + which-key--key-description< 1 0% + which-key--extract-key 1 0% + advice--make-docstring 1 0% + which-key--maybe-add-docstring 1 0% + which-key--list-to-pages 1 0% + which-key--echo 1 0% + delete-window 1 0% + consult--buffer-sort-visibility 1 0% + abbreviate-file-name 1 0% + powerline-render 1 0% + # 1 0% + timerp 1 0% + # 1 0% + complete-with-action 1 0% + seq-contains-p 1 0% + mapc 1 0% + vertico-sort-history-length-alpha 1 0% + jit-lock-context-fontify 1 0% + marginalia-annotate-binding 1 0% + profiler-stop ```

profiler result w `consult-org-roam`

``` 8675 48% + consult-org-roam--remove-capture-dups 5386 30% + file-truename 1939 10% + file-relative-name 1200 6% Automatic GC 227 1% + # 147 0% + # 95 0% + # 64 0% + # 45 0% + consult-org-roam--buffer-list-without-dups 28 0% + read-from-minibuffer 13 0% + # 11 0% + # 7 0% + org-roam-buffer-list 6 0% + all-completions 5 0% + mapcar 4 0% + org-get-limited-outline-regexp 4 0% + apply 4 0% + # 3 0% + replace-regexp-in-string 3 0% + complete-with-action 3 0% + # 2 0% + org-roam-descendant-of-p 2 0% + consult--buffer-query 2 0% + org-element--current-element 2 0% + org-roam-file-p 2 0% + org-element-keyword-parser 2 0% + emacsql-escape-scalar 2 0% + abbreviate-file-name 2 0% + # 2 0% + marginalia--function-doc 2 0% + profiler-stop 1 0% + default-font-width 1 0% + org-roam-db--get-connection 1 0% + outline-on-heading-p 1 0% + org-element--collect-affiliated-keywords 1 0% + assq-delete-all 1 0% + smartparens-strict-mode 1 0% + vertico--update 1 0% + # 1 0% + sort 1 0% + substitute-command-keys 1 0% + timer-activate-when-idle 1 0% + marginalia-annotate-binding 1 0% + timer--time-less-p 1 0% + timer-event-handler ```

In both cases, I start the profiler, call consult-buffer via C-x b, abort with C-g and stop the profiler.

I do have a lot of buffers (roam's: 265) open at the same time so looking into consult-org-roam--buffer-list-without-dups: https://github.com/jgru/consult-org-roam/blob/b93cc856d1d2dd0719926e51c5a6309bbbf60b19/consult-org-roam-buffer.el#L95-L103

it seems to do a string match on (length (org-roam-buffer-list))^2 (most of them are not duplicates).

It should probably faster to just look for CAPTURE-.* first then look for the original buffer? Tough I don't fully understand the description of consult-org-roam--buffer-list-without-dups (seems a word is missing) so maybe something evades me.

[jgru]

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

[jgru]

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

[aavanian]

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?

[jgru]

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

[aavanian]

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

[jgru]

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 96c95e5a14378cc6fc9c22981f5bfd9e18c419f6. #21 is closed now. I hope you enjoy using consult-org-roam.

Best regards, jgru