-
Notifications
You must be signed in to change notification settings - Fork 69
Improve performance by short-circuiting wrapping indents at a certain point #393
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
Conversation
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. |
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.
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?
As far as I can tell, my code calls |
Yes, they are disjoint. They can be anywhere on the line.
That's literally what this file is meant to solve.
You forgot to add an if x == 0 then
goto continue;
if x ~= 1 then
That's my bad, I didn't see |
@itsjunetime I feel like you misunderstood what It looks at a line and checks where it gets visually wrapped(see |
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 If you want to try it out now and let me know what you think, that would be great. |
I am still getting the feeling that we are not on the main page. Normally, when
I am looking at the code and I don't see how you are handling 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. |
As far as I can tell, 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? |
I have tested the PR and here are the nit-picks,
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 So, this might not get merged. Thanks for your effort though! |
This is an issue even on main, from my testing. So not a regression at all.
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 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. |
@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. |
I'm sorry, I'm still seeing the same lag in the I also collected a trace from my branch, if that's useful to you - the |
Can you share the text/file where the lag is happening? |
The file I'm using is a work-related doc, but I randomized it with |
Based on #393. Co-authored-by: itsjunetime <[email protected]>
@itsjunetime I have completely removed the need for It should be much faster now(and should work on text outside of the screen too). |
Beautiful, the |
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 thiswrap_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.