Skip to content

Conversation

itsjunetime
Copy link
Contributor

I was noticing some really annoying slowness on main today, so I finally decided to poke at it. After profiling with profile.nvim, it seemed that the vim.fn.screenpos in this wrap_indent loop was getting hammered a lot and that was taking up a lot of cpu time.

So to fix that, I just made this loop short circuit when we hit a character that is past position 1 in the line. As far as I can tell, this function only indents at the beginning of the line, so once we hit a character that's not displayed at the first index, we can be confident that we're done with indents for this line, and we can just return.

This PR also makes some other micro-optimizations, such as computing things once instead of twice a few times.

From some basic testing of markdown files, this completely gets rid of the performance problem while keeping everything looking the same. Someone else should probably look over it to make sure I'm not crazy, though; I don't write lua very often.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 23, 2025

As far as I can tell, this function only indents at the beginning of the line

This is incorrect as a line can be indented multiple times depending on how long the line is and/or how narrow the window is.


Also, your commit has a syntax error(see line 24).

So, it wouldn't even load.

And you are setting the extmarks twice.

@itsjunetime
Copy link
Contributor Author

This is incorrect as a line can be indented multiple times depending on how long the line is and/or how narrow the window is.

But are the indents disjointed? This function only indents at the beginning of the line, so we shouldn't need to handle a situation where there is an indent, normal text, and then another indent, all within a single line.

Also, your commit has a syntax error(see line 24).

So, it wouldn't even load.

I didn't modify line 24 at all, and I'm using my code right now in neovim, so it definitely loads. What do you see going wrong on your end?

And you are setting the extmarks twice.

As far as I can tell, my code calls nvim_buf_set_extmark exactly the same amount of times in each codepath as the previous code. Where are you seeing it being set twice?

@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 23, 2025

But are the indents disjointed?

Yes, they are disjoint. They can be anywhere on the line.

so we shouldn't need to handle a situation where there is an indent, normal text, and then another indent, all within a single line.

That's literally what this file is meant to solve.

I didn't modify line 24 at all, and I'm using my code right now in neovim, so it definitely loads.

You forgot to add an elseif here,

    if x == 0 then
      goto continue;
    if x ~= 1 then

Where are you seeing it being set twice?

That's my bad, I didn't see extmark_opts.id = extmark[1]; part.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 23, 2025

@itsjunetime I feel like you misunderstood what wrap.lua does.

It looks at a line and checks where it gets visually wrapped(see :h 'wrap'). And applies a fake indent there.

@itsjunetime
Copy link
Contributor Author

itsjunetime commented Sep 25, 2025

When I was first looking through this, I thought this function operated on an individual visual line within the grid, as opposed to a newline-character-delimited line. It makes more sense now.

I've updated my branch so it now does a rough search through the line to find the spaces in which it needs to insert indent characters. It doesn't move linearly, character-by-character, but instead it jumps forward by the width of a visual line each time it's done adding indents for one visual line. This significantly decreases the amount of times we have to call screenpos, making this run a lot better.

If you want to try it out now and let me know what you think, that would be great.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 25, 2025

I am still getting the feeling that we are not on the main page.

Normally, when wrap is enabled a long line will look something like this,

Screenshot_2025-09-25-15-03-32-136_com termux-edit

wrap.lua will turn it into this,

Screenshot_2025-09-25-15-03-49-433_com termux-edit


I've updated my branch so it now does a rough search through the line to find the spaces in which it needs to insert indent characters.

I am looking at the code and I don't see how you are handling conceal & virtual_text.

Note

Rough estimates leave edge cases, which was why the original file checked every character.

They can appear anywhere on the line and can change where a line gets wrapped.

@itsjunetime
Copy link
Contributor Author

As far as I can tell, vim.fn.screenpos handles conceal for us. The code in this PR works perfectly fine with, for example, markview's hiding of links.

Have you tried out this PR at all? Would you please see how it works for you and let me know of any specific, actionable fixes I could apply to make it mergeable?

@itsjunetime
Copy link
Contributor Author

I think this PR actually fixes an issue with wraps on main; this is what some indented bullet list looks like right now:

markview_main

Notice how the with is split up, but the second half of it isn't indented correctly. With this PR, it looks like this:

markview_pr

@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 26, 2025

I have tested the PR and here are the nit-picks,

  • It still can't handle indentation properly if there are regular spaces in front of the node.

Screenshot_2025-09-26-06-39-29-918_com termux-edit

  • The wrap breaking issue is still here. If I remember correctly, this is due to how screenpos() works. So, I don't think this will be fixed.

Screenshot_2025-09-26-06-29-48-936_com termux-edit

  • I have tested this on lazy.nvim's CHANGELOG.md file and the stuttering is still there(at least on my phone).

Also, just a heads up, I have been working on OXY2DEV/markview.nvim:optimize to optimize the plugin.

And this is one of the things I will be working on. I am hoping to not rely on screenpos() as much(cause it doesn't work for stuff outside of the screen).

So, this might not get merged.

Thanks for your effort though!

@itsjunetime
Copy link
Contributor Author

It still can't handle indentation properly if there are regular spaces in front of the node.

This is an issue even on main, from my testing. So not a regression at all.

The wrap breaking issue is still here. If I remember correctly, this is due to how screenpos() works. So, I don't think this will be fixed.

When I view this file (markview's README) with this PR, I don't see any difference from main. All the broken indents that show in your screenshot look correctly-rendered, with all the right indents, on my machine (except for one line, the line that says So, it must be loaded *before nvim-treesitter., but that is broken on main as well).

Maybe neovim has some weird difference in behavior on the version running on your phone versus a desktop? I don't know.

If you're not interested in merging this, though, feel free to close it I guess.

OXY2DEV added a commit that referenced this pull request Sep 26, 2025
@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 26, 2025

@itsjunetime I have reworked on the wrap module.

It should be better now. It also gets rid of the unnecessary function calls.

Give it a shot.

@itsjunetime
Copy link
Contributor Author

I'm sorry, I'm still seeing the same lag in the optimize branch - the screenpos calls are still causing a lot of lag. I can share the trace I have, showing each markview.renderer.render pass taking ~200ms, mostly made up by ~10k screenpos calls.

I also collected a trace from my branch, if that's useful to you - the markview.renderer.render calls take ~18ms and each have ~200 screenpos calls.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 26, 2025

Can you share the text/file where the lag is happening?

@itsjunetime
Copy link
Contributor Author

The file I'm using is a work-related doc, but I randomized it with awk '{gsub(/[a-zA-Z]+/,rand())}1 to produce:

text_file_randomized.md

OXY2DEV added a commit that referenced this pull request Sep 27, 2025
@OXY2DEV
Copy link
Owner

OXY2DEV commented Sep 27, 2025

@itsjunetime I have completely removed the need for screenpos.

It should be much faster now(and should work on text outside of the screen too).

@itsjunetime
Copy link
Contributor Author

Beautiful, the optimize branch runs so well now. Thanks so much for working on that! I'll close this PR since it's not necessary now.

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

Successfully merging this pull request may close these issues.

2 participants