neorg icon indicating copy to clipboard operation
neorg copied to clipboard

feat: parallel rendering of latex snippets

Open jonboh opened this issue 10 months ago • 11 comments

Hi! I've been using the latex renderer for some of my notes and I noticed its performance scaled badly with the amount of equations in the document. In short, equations currently are rendered one by one, in a blocking manner. This PR performs all the renderings in parallel.

Here is an example of the difference: Current main behavior (around 13s until the user gets back control): https://github.com/nvim-neorg/neorg/assets/31407988/f004090c-92bb-4f67-8187-d31997b05b27 This pr (3 to 4s until the user gets back control): https://github.com/nvim-neorg/neorg/assets/31407988/1104073e-700b-44c1-a5b1-e41abf2592c3

I've made a bunch of changes to the latex renderer that I think improve its usability:

  1. Refactor latex_renderer so that all latex_snippets are extracted and rendered in parallel jobs. This way we only need to wait for the longest render.
  2. Name the rendered files according to the base64 encoding of the snippet, this way subsequent calls to render_latex do not need to perform the conversion from latex to png. ~~I've added the same base64 utility function that is used in image.nvim. Let me know if it would be better placed in another location in the project or if it is better to use other implementation (I'm not very familiar with the lua ecosystem)~~.

EDIT: Point 3 is no longer part of this PR, as @benlubas correctly pointed out that the performance problems that I was experiencing with OnCursorMove only apply when you don't set the conceal option, I keep the text here for context: 3. Remove the "OnCursorMove" event handling. Unfortunately it is too costly whenever you have more than 5 or 6 equations on screen. If you try to open a document with ~10 equations like in the example you can end up with the nvim ui effectively locked if you try to move your cursor too quickly. In addition this event does not prevent the user from having to re-run latex_render if they are making modifications to the equations, as new equations won't get rendered on OnCursorMove event. I think it is better to just make the user re-run latex_render, but I'm open to revert this change if you consider it's still better to have the event as it is on main.

jonboh avatar Mar 31 '24 18:03 jonboh

@jonboh could you upload the large latex file? I've been working on improving this module as well and I'd like to test with a large file b/c currently my implementation doesn't hang neovim at all.

benlubas avatar Mar 31 '24 18:03 benlubas

sure, here's the file I've been using for testing:

* Test
  $\min_\theta \mathbb{E} -\log p_\theta(x)$
  $\min_\theta \mathbb{E} -\log p_\theta(x)$
  $e^{i\pi} + 1 = 0$
  $a^2 + b^2 = c^2$
  $E = mc^2$
  $F = ma$
  $\hat{H}\psi = E\psi$
  $\nabla \cdot \mathbf{E} = \frac{\rho}{\varepsilon_0}$
  $\nabla \cdot \mathbf{B} = 0$
  $dS \geq \frac{dQ}{T}$
  $\gamma = \frac{1}{\sqrt{1 - \frac{v^2}{c^2}}}$
  $(i\gamma^\mu\partial_\mu - m)\psi = 0$

jonboh avatar Mar 31 '24 18:03 jonboh

@benlubas have you uploaded your changes to your fork? I'll be happy to give them a try If you don't mind, there are still some things in this pr that don't work great, wheneve the images are moved by a line there's a slight delay, which is noticeable if you keep pressing Down, but I think that is related to image.nvim.

jonboh avatar Mar 31 '24 18:03 jonboh

I'm not sure if you're testing on nightly with the inline conceal, but when I use this branch, this happens after rendering and then making a change to the text: image

benlubas avatar Mar 31 '24 18:03 benlubas

I'm in the middle of rewriting mine to fix the move up/down flickering issue.

I can go grab a revision that's working and link it here for you to try though! It takes advantage of a change that was just merged into image.nvim yesterday, so make sure you update image.nvim.

https://github.com/benlubas/neorg/tree/feat/async_images

Here, this should work actually I haven't pushed anything breaking. I will warn you that with a large number of images it starts to get really really slow, it's definitely still a WIP.

The things that are working: inline images with conceal. conceal/unconceal on cursor move. updates the images when you change text. Images are displayed accurately even if there are virtual lines above the image.

benlubas avatar Mar 31 '24 19:03 benlubas

I'm trying the conceal again and surprisingly a lot of the performance problems that appear with several equations are not present this way... I'm going to re-add the OnCursorMove, as it seems to work well, and in that case the clearing after text edits works ok. The sequential processing of the latex still offers quite a speed-up. I'll try to check your branch tomorrow and see if we have common things that could work together.

jonboh avatar Mar 31 '24 19:03 jonboh

My branch is going to change a lot in a few days, I might work on it a bunch more today now though

benlubas avatar Mar 31 '24 19:03 benlubas

I think we shouldn't write a base64 module here We should rather use vim.base64 from this pr https://github.com/neovim/neovim/pull/25843 which likely is just as fast since it's written in C

max397574 avatar Mar 31 '24 19:03 max397574

I wasn't aware of vim.base64, thanks, that makes things easier. I've removed the base64 file.

jonboh avatar Mar 31 '24 21:03 jonboh

I've added as well a fix to handle cases in which the math snippet does not compile to an image, for example $\e$. In main this case throws an "attemp to index a nil value".

Now a small log message will be generated indicating which snippet could no be compiled.

jonboh avatar Apr 01 '24 17:04 jonboh

@jonboh Just opened a PR for image.nvim that can fix a lot of the performance issues I was having. It should also fix your performance issues. https://github.com/3rd/image.nvim/pull/141

try it out, and let me know how it goes. (it might break something, but I couldn't think of anything that would break)

benlubas avatar Apr 04 '24 03:04 benlubas

Sorry your work got superceded, I forgot to tag you here and close the issue. Thanks for all the work you put in anyway :)

vhyrro avatar May 27 '24 19:05 vhyrro