-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Semantic highlighting optimisation #4150
Semantic highlighting optimisation #4150
Conversation
addf4f5
to
6ecb7b4
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4150 +/- ##
==========================================
+ Coverage 86.14% 88.84% +2.69%
==========================================
Files 33 34 +1
Lines 4389 4393 +4
==========================================
+ Hits 3781 3903 +122
+ Misses 608 490 -118 |
Re-use the "optimal range" drawing used by inlay hints, i.e. only re-request if the new visible range is not covered by the previously requested visible range. Essentially, semantic highlighting and inlay hints now use the (almost) exact same code. Still TODO (for both): record the _actual_ received range and compare against that. The point is that we should only redraw if the new range isn't within the last _drawn_ range. This is important for semantic highlighting because servers might not support range highlighting and we should therefore paint the whole buffer and only re-do that when it changes.
This means that if the servers return a wider range of semantic tokens or inlay hints than we requested, we don't constantly re-request them whenever the viewport moves. Importantly, clangd doesn't support range tokens, so we previously requested and re-parsed, deleted and re-added every single token on every single scroll event. This meant that scrolling was very slow and lots of wasted effort. This should help reduce that. We should essentially re-use the tokens on scolling and pretty much never re-request them unless the buffer changes or we have a file-ready-to-parse.
6ecb7b4
to
724218e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to test this for a bit in practice but them merge
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @puremourning)
Scrolling behaviour much improved. however, there are still forceful requests happening that aren't really needed, such as when doing Really should finally write some tests for these features. |
Clangd now includes all stdlib stuff by default in the initial search, so we sometimes match random stdlib symbols. Make the symbols we search for a little more unique by prefixing with x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 7 files at r2, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @puremourning)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @puremourning)
Thanks for sending a PR! |
Minimise overdraw and over-request for semantic tokens.
Share the scrolling window implementation with inlay hints which was already a bit better.
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)