Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More performant rendering/activation of citations #842

Open
krisbalintona opened this issue Sep 16, 2024 · 7 comments
Open

More performant rendering/activation of citations #842

krisbalintona opened this issue Sep 16, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@krisbalintona
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I notice that the rendering/activation of org-mode citations with citar (i.e. when org-cite-activate-processor is 'citar) is very slow. When editing a line with one or more citations, Emacs freezes for multiple seconds on every character inserted or deleted. This is even the case for small .bib files.

Describe the solution you'd like
When editing (inserting or deleting characters) a line with citation(s), render citations as quickly as possible so that Emacs does not freeze.

Describe alternatives you've considered

  1. Other packages such as org-cite-activate-processor that activate citations differently. Different packages have different pros and cons to how they activate citations --- and having to install another package is not much of a solution with respect to citar. I did not like this solution.
  2. I defined by own activate function and replaced the org-cite-basic-activate in citar-org-activation-functions with it. My version, below, is identical to org-cite-basic-activate but with faster rendering by using citar's functions which also leverage its cache (I believe). I currently use this approach.
   (defun kb/citar-basic-activate (citation)
    "Set various text properties on CITATION object.

Fontify whole citation with `org-cite' face.  Fontify key with `error' face
when it does not belong to known keys.  Otherwise, use `org-cite-key' face.

Moreover, when mouse is on a known key, display the corresponding bibliography.
On a wrong key, suggest a list of possible keys, and offer to substitute one of
them with a mouse click.

My version of this function uses the speed of `citar' (and its cache) to
eliminate the extraordinary slow down of the default function's
rendering. I believe the bottlenecks are `org-cite-basic--all-keys' and
`org-cite-basic--get-entry' so I have replaced them with equivalent
functions from `citar'."
    (pcase-let ((`(,beg . ,end) (org-cite-boundaries citation))
                ;; NOTE 2024-09-05: Use `citar' (and its cache) to get all keys
                (keys (let (keys)
                        (maphash (lambda (key value) (push key keys))
                                 (citar-get-entries))
                        keys)))
      (put-text-property beg end 'font-lock-multiline t)
      (add-face-text-property beg end 'org-cite)
      (dolist (reference (org-cite-get-references citation))
        (pcase-let* ((`(,beg . ,end) (org-cite-key-boundaries reference))
                     (key (org-element-property :key reference)))
          ;; Highlight key on mouse over.
          (put-text-property beg end
                             'mouse-face
                             org-cite-basic-mouse-over-key-face)
          (if (member key keys)
              ;; Activate a correct key. Face is `org-cite-key' and `help-echo'
              ;; displays bibliography entry, for reference. <mouse-1> calls
              ;; `org-open-at-point'.
              ;; NOTE 2024-09-05: Use `citar' (and its cache) to create the
              ;; bibliographic entry text used in the help echo
              (let* ((entry (string-trim (citar-format-reference (list key))))
                     (bibliography-entry
                      (org-element-interpret-data entry)))
                (add-face-text-property beg end 'org-cite-key)
                (put-text-property beg end 'help-echo bibliography-entry)
                (org-cite-basic--set-keymap beg end nil))
            ;; Activate a wrong key. Face is `error', `help-echo' displays
            ;; possible suggestions.
            (add-face-text-property beg end 'error)
            (let ((close-keys (org-cite-basic--close-keys key keys)))
              (when close-keys
                (put-text-property beg end 'help-echo
                                   (concat "Suggestions (mouse-1 to substitute): "
                                           (mapconcat #'identity close-keys " "))))
              ;; When the are close know keys, <mouse-1> provides completion to
              ;; fix the current one. Otherwise, call `org-cite-insert'.
              (org-cite-basic--set-keymap beg end (or close-keys 'all))))))))
  (setq citar-org-activation-functions '(kb/citar-basic-activate citar-org-activate-keymap))

Additional context
I didn't have enough time to be 100% why org's built-in org-cite-basic-activate is slow, but I noticed that merely replacing org-cite-basic--get-entry and citar-get-entries with citar equivalents in the activate function does wonders with respect to performance on my machine.

@krisbalintona krisbalintona added the enhancement New feature or request label Sep 16, 2024
@rudolf-adamkovic
Copy link

+1 I also suffer from bad performance. When scrolling large buffers with lots of citations, Emacs regularly locks up for seconds when Citar is enabled.

@bdarcus
Copy link
Contributor

bdarcus commented Sep 17, 2024

@andras-simonyi - any thoughts on this? It's related to you your activate processor.

@andras-simonyi
Copy link

andras-simonyi commented Sep 22, 2024

@andras-simonyi - any thoughts on this? It's related to you your activate processor.

I'm not sure, I have tested now oc-csl-activate on a 544K Org file with more than 1200 citations and haven't noticed any rendering slowdown during editing with or without using the citar cache as information source.

@krisbalintona
Copy link
Contributor Author

@andras-simonyi - any thoughts on this? It's related to you your activate processor.

I'm not sure, I have tested now oc-csl-activate on a 544K Org file with more than 1200 citations and haven't noticed any rendering slowdown during editing with or without using the citar cache as information source.

I'd like to clarify, your oc-csl-activate does it's job well — I get no slowdowns there. I only referenced it in my original post because although a user can install another package to alleviate performance, I think the default activate function for citar shouldn't be slow to begin with. The function I included in my original post is one that should basically be identical to citar's current functionality but without the slowdown.

@bdarcus
Copy link
Contributor

bdarcus commented Sep 22, 2024 via email

@bdarcus
Copy link
Contributor

bdarcus commented Sep 23, 2024 via email

@krisbalintona
Copy link
Contributor Author

The current one IIRC just reuses the default org one. If that's the case, ideally the fix would be made there?

You do have a point: if org-cite-basic-activate was faster, citar would have no issue. However, I'm not sure if there's a painless solution to fixing that function without citar (though I'm not versed in the oc.el and friends codebase). Citar provides performant functions for the task, so at least in the meantime I think leveraging those functions for users of citar would be best — the functionality is identical anyway.

(I'll consider officially bringing this up to the org-mode devs, but to be frank, I'm only familiar with contributing and discussing on github, so the task is daunting.)

I'll look at this more closely soon. It does seem appropriate for here, given use of the cache. A PR would be nice.

Sure, I can open a PR on this if you think it's worth pursuing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants