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

Wrapped long line and folding performance #513

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

karlvr
Copy link

@karlvr karlvr commented Jun 24, 2023

This is related to #159 #41.

This PR contains a few optimisation suggestions for RSyntaxTextArea, which have made an enormous difference to the performance of wrapping large single-line text files (e.g. minified JavaScript or JSON).

The first commit is a simple cache of character position in a wrapped line to line height, so we don't have to measure from the start of the string every time.

The second commit avoids measuring widths at all if we don't need to (there were places where the measurement wasn't used), and importantly, to measure only up to a maximum value if that would flip us into an alternative code path that didn't need the accurate result, e.g. in calculateBreakPosition where if the measured width is greater than the width of the text area we don't care how wide it actually was.

I think the results are still accurate!

I say this PR is a suggestion as I have simply duplicated methods in TokenImpl and the parameter names aren't A+. But if you like it @bobbylight then I'll happily tidy it up, tidy it up with you, or hand it over to you as per your preference.

Finally, thank you very much for this excellent project.

@karlvr
Copy link
Author

karlvr commented Jun 24, 2023

There is also a rendering performance issue in WrappedSyntaxView.drawView (and the highlighted equivalent) where if we have a large View that perhaps contains thousands of tokens, we draw all of the chars in it every time we paint, rather than using the clip rectangle to limit drawing.

I have pushed a couple of commits to resolve that issue and now I'm scrolling smoothly in a view with no syntax highlighting but a huge long line.

A View can be enormous in the case of a massive unbroken line.
I’ve observed ongoing one pixel changes backwards and forwards in width in the setSize method from its two different call-sites.
@karlvr
Copy link
Author

karlvr commented Jun 24, 2023

The "avoid spurious resizes" commit I think might be indicative of something wrong going on elsewhere, and is most likely the wrong fix for this issue. That will bear some more investigation

The next issue I ran into was the performance of folding on large JavaScript files, so I've added a commit to improve the performance of folding. I think I've made OK assumptions re using the same binary search approach from getFoldForLineImpl in isLineHiddenImpl... I ran the old and new algorithms in parallel on a large JavaScript file and they gave the same results... but it's always nerve-racking with off-by-one-prone code like this (e.g. the first line of a fold is visible).

@karlvr karlvr changed the title Wrapped long line performance Wrapped long line and folding performance Jun 24, 2023
@FSchumacher
Copy link

I have tested your branch with JMeter (which has problems, when tokens way longer than one line are displayed, see #159 for example). With your patches, the problems are gone and we could throw away all our workarounds.

Any chance, this could be incorporated into the main branch?

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